trying to implement pgsqlencoder

Question

trying to implement pgsqlencoder

Opened this issue 8 years ago · 22 comments

On the "actual" thing , some info on what happens
running this with the current implementation

    $person = new Person([
        'person_fname' => 'lapo',
        'person_lname' => 'kapo',
        'phone1' => '2810222333',
        //  'comments'=>[
        'comments.tags' => ["tag1" => 1, "tag2" => 3, "tag3" => ["trialari" => "lari", "tralalalali" => "lolo"]],
        'comments.title' => "this title",
        'comments.author' => "thisauthor",
        'comments.lalalal.la.la' => "lalala11",
        'comments.binstring' => "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2a\x2b\x2c\x2d\x2e\x2f\x30\x31\x32\x33\x34\x35\x36\x37\x38\x39\x3a\x3b\x3c\x3d\x3e\x3f\x40\x41\x42\x43\x44\x45\x46\x47\x48\x49\x4a\x4b\x4c\x4d\x4e\x4f\x50\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5a\x5b\x5c\x5d\x5e\x5f\x60\x61\x62\x63\x64\x65\x66\x67\x68\x69\x6a\x6b\x6c\x6d\x6e\x6f\x70\x71\x72\x73\x74\x75\x76\x77\x78\x79\x7a\x7b\x7c\x7d\x7e\x7f\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8a\x8b\x8c\x8d\x8e\x8f\x90\x91\x92\x93\x94\x95\x96\x97\x98\x99\x9a\x9b\x9c\x9d\x9e\x9f\xa0\xa1\xa2\xa3\xa4\xa5\xa6\xa7\xa8\xa9\xaa\xab\xac\xad\xae\xaf\xb0\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xba\xbb\xbc\xbd\xbe\xbf\xc0\xc1\xc2\xc3\xc4\xc5\xc6\xc7\xc8\xc9\xca\xcb\xcc\xcd\xce\xcf\xd0\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xdb\xdc\xdd\xde\xdf\xe0\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff"
        //],
        // 'comments.author.thisauthor'=>["tralala","lalala"]

    ]);
    $person->save();

returns this

SQLSTATE[42P18]: Indeterminate datatype: 7 ERROR: could not determine data type of parameter $7
The SQL being executed was:

INSERT INTO "person" (
    "person_fname",
    "person_lname",
    "phone1",
    "created_at",
    "updated_at",
    "created_by",
    "updated_by",
    "person_id",
    "comments"
) VALUES (
    'lapo',
    'kapo',
    '2810222333',
    NOW(),
    NOW(),
    1,
    1,
    '00ce9db2-8dcb-4004-b132-d061a6188c93',
    json_build_object('comments',
        json_build_object('tags',
            json_build_object('tag1', 1, 'tag2', 3, 'tag3',
                json_build_object('trialari','lari','tralalalali','lolo')::jsonb
            )::jsonb,
            'title',
            'this title',
            'author',
            'thisauthor',
            'lalalal',
            json_build_object('la',
                json_build_object('la','lalala11')::jsonb
            )::jsonb,
            'binstring',
            'data:application/octet-stream;base64,AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0+P0BBQkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWltcXV5fYGFiY2RlZmdoaWprbG1ub3BxcnN0dXZ3eHl6e3x9fn+AgYKDhIWGh4iJiouMjY6PkJGSk5SVlpeYmZqbnJ2en6ChoqOkpaanqKmqq6ytrq+wsbKztLW2t7i5uru8vb6/wMHCw8TFxsfIycrLzM3Oz9DR0tPU1dbX2Nna29zd3t/g4eLj5OXm5+jp6uvs7e7v8PHy8/T19vf4+fr7/P3+/w=='
        )::jsonb
    )::jsonb
) RETURNING "person_id"

Error Info: Array
(
    [0] => 42P18
    [1] => 7
    [2] => ERROR:  could not determine data type of parameter $7
)

now if we do not call dynColSqlMaria (it is altered by nineinchnick to create jsonb_objects) and simply json_encode like this

public function encodeDynamicColumn($attributes)
{
    if (!$attributes) {
        return null;
    }

    $params = [];

    // todo For now we only have Maria. Add PgSQL and generic JSON.          static::encodeDynamicAttributeArray($attributes);
    $sql = json_encode($attributes); //simply encode attributes
    //   $sql = static::dynColSqlMaria($attributes, $params);
    $sql = '\'' . $sql . '\''; //simply add ' ' before and after so pg accepts the value
    return new \yii\db\Expression('(select CAST (' . $sql . ' AS JSONB))', $params);
    // return new \yii\db\Expression($sql,$params);     
}

it returns this and gets inserted in the db. I do not know what side effects there will be since I cannot run the tests I would like some lecturing on that part :-)
this is the info I could gather

{
    "comments": {
        "tags": {
            "tag1": 1,
            "tag2": 3,
            "tag3": {
                "trialari": "lari",
                "tralalalali": "lolo"
            }
        },
        "title": "this title",
        "author": "thisauthor",
        "lalalal": {
            "la": {
                "la": "lalala22"
            }
        },
        "binstring": "data:application/octet-stream;base64,AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0+P0BBQkNERUZHSElKS0xNTk9QUVJTVFVWV1hZWltcXV5fYGFiY2RlZmdoaWprbG1ub3BxcnN0dXZ3eHl6e3x9fn+AgYKDhIWGh4iJiouMjY6PkJGSk5SVlpeYmZqbnJ2en6ChoqOkpaanqKmqq6ytrq+wsbKztLW2t7i5uru8vb6/wMHCw8TFxsfIycrLzM3Oz9DR0tPU1dbX2Nna29zd3t/g4eLj5OXm5+jp6uvs7e7v8PHy8/T19vf4+fr7/P3+/w=="
    }
}

Answer 1 · 2017-01-26T21:37:14.000Z

Thanks for the report @gvasilopulos.

I reformatted your post to make it more readable.

Even so, I'm not sure what the problem is. Can you reduce this to a simple example with only one or two object fields?

Answer 2 · 2017-02-01T07:09:13.000Z

Sorry for the delay I've been closing shop/moving things from shop to house and been pretty busy.
Some more info, read for DAR seems to work more or less.
Reading dar fields seems to work with $model->getAttribute().
In views they get back as expected.

They (fields) cannot get used in forms in their current dotted form so I guess some workaround should be used, since $field($model, dotted.attribute) is not accepted, but I guess that is normal.
Update works too with $model->setAttribute() without messing with the rest of the json. DAQ is not tested yet.

I do not know if you want me to sent you any related files which I used to do the tests (which are not extended at the moment) if you do, please let me know I 'll be happy to provide anything that you consider necessary(db,project,whatever).

Still cannot run the tests, Pg test is kind of empty, but cannot run them even for maria, namespaces seem to be messed up with copying tests folder from github and placing it in yii-dev package,tried to fix autoloads but something seems to be missing. Will try to fix that asap.
DAQ is untested will do asap too.

This is what I used to update 2 of the json objects in the json field (comments)

 public function actionEdit($id)
       {
        $model = $this->findModel($id);
        $model->setAttribute('comments.tags.tag2',56.2);
        $model->setAttribute('comments.author','Nikos Kazantzakis');
        if ($model->loadAll(Yii::$app->request->post()) && $model->saveAll()) {
            return $this->redirect(['view', 'id' => $model->person_id]);
        } else {
            return $this->render('update', [
                'model' => $model,
            ]);
        }
    }
 }

And this is how I got the related fields values in a view
    $gridColumn = [
        [
            'attribute'=>'comments.author',
            'value'=>$model->getAttribute('comments.author'),
        ],
        [ 'attribute'=>'comments.tags.tag2',
            'value'=>$model->getAttribute('comments.tags.tag2'),],
    ];
    echo DetailView::widget([
            'model' => $model,
            'attributes' => $gridColumn
        ]);

Answer 3 · 2017-02-01T14:14:54.000Z

They (fields) cannot get used in forms in their current dotted form

The docs say "The dot notation works anywhere Yii accepts an attribute name string, for example" and then shows using it in rules(). So this in itself is a bug.

@gvasilopulos Can you enter this as a separate issue with a simple test case and the evidence of how it fails (errors, logs)?

Answer 4 · 2017-02-01T21:50:18.000Z

to get the unit tests to work i deleted the entire vendor dir, composer.lock and did composer update --prefer-source

then check that this line

Yii::setAlias('@yiiunit', __DIR__ . '/../../vendor/yiisoft/yii2-dev/tests');

points to an existing directory in vendor/yiisoft/yii2-dev/tests like this

@gvasilopulos try like this and if it still fails, push your work to your fork and tell me the steps i need to take to see the errors that you see

Answer 5 · 2017-02-01T22:45:47.000Z

to get the unit tests to work i deleted the entire vendor dir, composer.lock and did composer update --prefer-source

since 2.0.11 is now released, a simple composer update should work.

Answer 6 · 2017-02-02T07:11:48.000Z

My bad, did not use rules for some strange reason I thought a generic rule ['dynamic_column' , 'safe']
would be sufficient. Seems not. update and create seems to work with rules as defined by you. I think next step would be to fix the test issue and take it from there.

Answer 7 · 2017-02-02T07:13:11.000Z

@tom-- , @cebe thank you both for your responces!

Answer 8 · 2017-02-02T22:12:52.000Z

ok runned the tests for postgresql and been having some strange failures with unique constraints violation, I removed my test db and recreated it just in case, but still unique constraints violation remain
see here https://github.com/gvasilopulos/yii2-dynamic-ar/blob/pgsql-encoder/pgsql1.txt multiple failures because of this. Other tests failed because they are simply not supported by json spec e.g. single quote in strings. Other test failures need a little research. There are various limitations on json spec regarding what is accepted and pg is strict about this. Probably will have to adjust the tests for json. Tom if you can make time please have a look on json.org and tell me what should be excluded from the tests. /u0000 is a no go for pg too so that definitely remains excluded.

Answer 9 · 2017-02-03T17:03:42.000Z

@gvasilopulos where is your repo so we can try this for ourselves?

both issues may be in tests/unit/data/pgsql.sql.

it has a bunch of JSON docs in string literals
the part above line 264 is probably inconsistent with changes that have happened in yii framework since this part was copied from vendor/yiisoft/yii2-dev/tests/data/ some time before 2015-09-03.

try...

copy vendor/yiisoft/yii2-dev/tests/data/postgresql.sql to tests/unit/data
append the lines from 264 to the end of tests/unit/data/pgsql.sql to tests/unit/data/postgresql.sql
update tests/unit/data/config.php to reference the new fixture file

see what happens

Answer 10 · 2017-02-03T19:52:33.000Z

the repo is this https://github.com/gvasilopulos/yii2-dynamic-ar/tree/pgsql-encoder
I'll try your suggestions and report back

Answer 11 · 2017-02-03T21:35:39.000Z

I have a feeling that line 264 is wrong, it is in the middle of an insert statement line 197 looks more like it. So I did it like this. But it didn't fix the problem, what I did is I removed the id from the insert statement and let pg decide through serial, if this approach is wrong then I should figure out why it tries to enter id=1 in every row it tries to insert..
After I removed the id from the insert statements, more tests succeded, the report is here:
https://github.com/gvasilopulos/yii2-dynamic-ar/blob/pgsql-encoder/pgtestresult_id_by_pg.teamcity
almost every find test fails, partialy because it is maria oriented, partialy because some tests are simply not supported. Should I rewrite pgoriented tests ?
Postgres uses jsonb operators to find keys and values within the jsonb field, If this can be supported by AR is unknown to me, but have a look at this http://schinckel.net/2014/05/25/querying-json-in-postgres/ and this https://www.postgresql.org/docs/9.5/static/functions-json.html

Answer 12 · 2017-02-03T22:33:18.000Z

I have a feeling that line 264 is wrong, it is in the middle of an insert statement line 197 looks more like it.

What file are you referring to?

Generally we use explicit ID values so that we get the same data in the tables every time. So long as we don't violate the PK constant, there should be no problem. And if there is, that suggests there's a different problem.

why it tries to enter id=1 in every row it tries to insert..

why do you think it does?

Answer 13 · 2017-02-03T22:36:19.000Z

I think first you should get yii's postgresql tests to work. If they all run properly and pass then you have a basis for elaborating the fixture for dynamic active record.

Trying to convert the maria stuff might not be the best way. It might be easier to follow the maria tests as a guide, copying those that should work, or writing something that tests the same kind of thing otherwise.

Answer 14 · 2017-02-03T22:39:11.000Z

After a quick look I imagine that all of DAQ's features can be supported by pgsql's jsonb query features with fairly direct translation.

Answer 15 · 2017-02-04T14:51:54.000Z

Hello I runned the pgtests which gave me this yiisoft/yii2#13511 but I do not think this is a reason not to go ahead.
I was referring to your earlier comments on fixing the test errors :
"try...

copy vendor/yiisoft/yii2-dev/tests/data/postgresql.sql to tests/unit/data
append the lines from 264 to the end of tests/unit/data/pgsql.sql to tests/unit/data/postgresql.sql
update tests/unit/data/config.php to reference the new fixture file

"
I think that proper line was 197, are we looking at the same pgsql.sql fixture ?
I 'll try to start working on DAQ and see what happens yes ?
The id issue was avoided with the previously mentioned workaround (skipped feeding id to pg). I do not consider it a solution, but do not feel like looking in to that right now, unless you consider it to be essential and needs to be addressed right away.

Answer 16 · 2017-02-04T23:34:49.000Z

I've made some changes on PgsqlEncoder.php and runned the tests again. In general we are on the right path. Most of the tests fail because postgres wants every json value that we check against the dynamic column to be quoted and strings to be quoted and double quoted. you can check the results in pgencoder.xml on my repo, if you run the failed queries with numerics as '2343.33' and strings as '"lala"' in the right clause of the where comparisons you will see that they will be accepted. Array tests seem to fail regardless. Please do have a look and throw some idea as how to move on. I think Pgencoder is on a good path now. I used jsonb_extract_path on dynamicAttributeExpression function, it seems to be generic enough for us. One question, are queries with LIKE working straightforward in maria? If this is the case I am afraid in pg this is not supported oob. There are ways to achieve that in sql but dunno how this could be implemented in DAQ/DAR

Answer 17 · 2017-02-06T23:40:29.000Z

I'll try to resemble something of a bug report :-)

I have modified PgsqlEncoder.php lines 22-33 to use jsonb_extract_path.

jsonb_extract_path returns the value found at the specific path as a jsonb postgres column , which
resembles a json object, this is very generic and up to a point a good thing.

To elaborate if something is a string it will be returned as "string" (double quoted) everything else returns normal, which means in the test assertions this lines 352-353 will not work

 $val = Product::find()->where(['id' => 1])->select(['(!children.str!)'])->scalar();
 $this->assertEquals('value1', $val);

while this lines 361-362 will work:

    $val = Product::find()->where(['id' => 1])->select(['(!children.str!)'])->scalar();
    $this->assertEquals('"value1"', $val);

if you remain in this function you will see that I altered the test to assert 'null' instead of assertNull and boolean to accept 'true' not 1 , this comes from json pg datatype and the test passes like this.
see here : https://www.postgresql.org/docs/9.4/static/datatype-json.html
I do not know if this is ok for the AR specs, but on the other hand we have json not SQL and I think
this is how json is implemented. Give me your opinion on this.

array tests all fail , partialy because I havent altered the test on array test but I thing even so , the array tests have another reason to fail, it is on the third paragraph of the above pg json type link.

To make a long story short it says jsonb json objects do not keep the order of keys but store them as they see fit for searching. I think this also gets in the way

The tests I used is [this](https://github.com/gvasilopulos/yii2-dynamic-ar/blob/pgsql-encoder/tests/unit/DynamicActiveRecordTestPgJson.php
And the results are here
https://github.com/gvasilopulos/yii2-dynamic-ar/blob/pgsql-encoder/pgencoder6.xml
I will give you a summary bellow:
Summary
Test file : https://github.com/gvasilopulos/yii2-dynamic-ar/blob/pgsql-encoder/tests/unit/DynamicActiveRecordTestPgJson.php
Result File : https://github.com/gvasilopulos/yii2-dynamic-ar/blob/pgsql-encoder/pgencoder6.xml
Failed Tests:
1 testAsArray()
line of test file : 91
line of results file: 7
reason: unknown
suspected reason: difference of boolean values and/or swapping of keys in jsonb
2 testWriteRead()
line of test file: 232
line of result file : 26 fail is in line 53
reason : unknown
suspected reason: same as the above
3 testDynamicValueObects()
line of test file: 254
line of result file: 71
reason:unknown
suspected reason : unknown
4 and 5 custom columns (2 tests)
line of test file: 315,326
line of result file:96,104
reason : unknown but probably for the same reason.
suspected reason : possible pdo missbehavior. I intentionally made an error in the sql code in another test run to see the produced sql query which (by fixing the intentional error) gives the expected result
so the produced query seems to be correct, I do not know how to debug any of this further. Any thoughts appreciated. Strange thing is complex condition test runs ok which produces a much more complex query.
6 testFindAsArray()
line of test file : 408
line of results file: 116
reason : unknown

useful notes
jsonb_extract_path returns value as a json, but can be casted to textual, if we do that we can do searches with like, and in general we can do anything we could do with a textual string in comparisons or equality checks or like/regexp searches, from textual we can also cast to numeric so we get a number back with functions as MIN MAX AVG made possible to run. good thing is that we can simply extend the dar field like this to do a type cast (!int!)::text::numeric

Answer 18 · 2017-02-07T17:11:15.000Z

If jsonb_extract_path returns JSON then it must be decoded before we proceed. There are two cases:

We select the dyn-col expression, in which case we need to decode it after we get it back in PHP
The dyn-col expression is part of a larger SQL expression, in which case we need to add decoding to the SQL statement.

Answer 19 · 2017-02-07T18:59:45.000Z

jsonb_extract_path is the same like #> I find that easy to implement but mostly, I try to avoid "operators", in the case we want to extend dar, or outright include in dar more pg json function, what led me to this decision is the containment operator which is ? and practicaly messes things in PDO and needs to be recreated as a custom operator to work.
Saw this stackoverflow post about this. http://stackoverflow.com/questions/30461558/jsonb-existential-operators-with-parameterised-queries
So I decided to use functions instead.

Wouldn't a simple json_decode before return do the trick ?
I guess I'll have to be compatible with DAQ syntax. I mean (!dyncol|numeric!)

Answer 20 · 2017-02-07T20:19:19.000Z

I expect so. You just need to know when to apply it and to what.
Perhaps use json_extract_path when the type is given and it is not string/char, otherwise json_extract_path_text

Answer 21 · 2017-02-12T12:58:19.000Z

news from the front :-)
jsonb_extract_path_text works sufficiently in most cases
I modified PgsqlEncoder to create the appropriate query lines 20 to 41.
I left the option to use jsonb so someone who for any reason wants to get the original json value from the db to be able to do so.
Please have in mind that I am not an experienced programmer so don't freak out :-).
Any suggestionson doing things a better way is appreciated. One thing that might strike you as strange is the explodein line 25 of pgsqlencoder.
https://github.com/gvasilopulos/yii2-dynamic-ar/blob/pgsql-encoder/encoder/PgsqlEncoder.php
This was done because of testCustomColumnsExpression test on line 322 of https://github.com/gvasilopulos/yii2-dynamic-ar/blob/pgsql-encoder/tests/unit/DynamicActiveRecordTestPgJson.php fails unless type is explicitly declared. could not think of something else..

I altered DynamicActiveQuery
https://github.com/gvasilopulos/yii2-dynamic-ar/blob/pgsql-encoder/DynamicActiveQuery.php
to use a different regexp for postgres. I do not know if this is the propper way to check for db driver.
I suspect that this will call db->getDriverName every time, suggestions on this are also appreciated.

Known Issues

testWriteRead on line 233 of
https://github.com/gvasilopulos/yii2-dynamic-ar/blob/pgsql-encoder/test/unit/DynamicActiveRecordTestPgJson.php fails.
I have a feeling that dataProviderTestMariaArrayEncoding needs to be altered to match possible jsonb limitations, I need to research this further.
Test testFindAsArray on line 402 fails line 411 gets checked against json ('{"int": 456}') , we probably should json_decode dynamic_columns beforehand but how ?
Parent test also fail for a different reason boolean values do not match. I think I have to review fixtures from latest yii repository but that does not affect the failure in the children test.
testFindScalar (line 344) in fact fails unless we json_decode($val) as is in line 353, or check against 'true'.
Suggestions on this are also welcome.

Answer 22 · 2017-02-17T08:46:22.000Z

On the above Issue, what fails is the first dataset of dataProviderTestMariaArrayEncoding which is [[1]]. line 120 of https://github.com/gvasilopulos/yii2-dynamic-ar/blob/pgsql-encoder/tests/unit/DynamicActiveRecordTestPgJson.php.
I think that there should be a key,value pair to encode a proper json column, key/index will not be created automaticaly in json.
I'm trying to wrap my head around this if it is okay to fail. It is checked against 1 not array.
On the issue we chatted about yesterday.
this is the error report : https://gist.github.com/gvasilopulos/5cbd88077e223ec2c902d7dd4c084a22
just to remind you about it. I have come to this conclusion.
First of all the assumption I stated that this was happening when the db did not have jsonb value was wrong. There was a jsonb value inside from tests I've done in the db previously (before I started to mess around DAR). This is normaly not a problem, but through a coincidence we have discovered a behavior that is not dissallowed by json, but I guess it is in maria's dyncol.
What is happening is this. Through the update I was trying to set a value of comments.title which normally is
'{"comments":{"title":"some string"}} .
Now in the db there is already a json key "comments" which is a string it's like {other_key_value_pairs, "comments":"string",other_key_value_pairs}.
So decoder was trying to find comments.title to decode as an array but there was no array there there was a string (yes you were right).
In jsonb column this is an acceptable change, which means that "comments":"string" will
be updated and replaced with "comments":{"title":"value"}.
We can either accept this as a valid limitation of the implementation or we could somehow ignore/bypass the error, and encode the given data for update.
There are other things that have to be taken in to account. We have to declare behaviors on the model to ensure proper handling of fields for boolean and number since the values are returned as string by POST/GET and there is no way so far for yii2 to autodetect the type from the db.
Other than these so far in real life test it seems to work as expected
Please share your thoughts.