-
-
Notifications
You must be signed in to change notification settings - Fork 171
JSON select support for sqlite #536
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
copied and added to sqlite folder and altered to use a different database and sql syntax. This also changes the qrm package for dealing with returned strings instead of bytes for json values in sqlite.
|
I forgot to change the date and time format statement from mysql syntax to sqlite syntax |
|
@oscar345 Nice work
Not much we can do about it. It seems that json real numbers are serialized differently in sqlite json compared to sqlite driver. To pass the test, we can unmarshal test data from
Since every nested json object has to be aliased, we can intercept alias call by implementing |
| } | ||
|
|
||
| value = []byte(valueStr) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this code is ever called in your tests. This case is active only when SELECT_JSON appears as a column inside regular SELECT statement - wiki.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, now that I check, it seems that this test is missing for mysql.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is needed though since I was using the select_json inside a regular select in my own code and otherwise there was convert error. I can copy the tests from postgres into sqlite to see check if all cases work correctly. The tests for mysql should probably be in another PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that makes sense.
| } | ||
|
|
||
| value = []byte(valueStr) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, now that I check, it seems that this test is missing for mysql.
| return CustomExpression(Token("strftime('%Y-%m-%dT00:00:00Z', "), e, Token(")")) | ||
| case BoolExpression: | ||
| return CustomExpression(Token("CASE"), e, Token("WHEN 1 THEN json('true') ELSE json('false') END")) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All this types needs to be covered by tests as well. Check TestAllTypesJSON test for mysql.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I will take a look at the mysql tests and add them to the sqlite tests as well
Should I just create a second file called
If I looking at the other stuff I will try to make this working but currently I am not totally sure if I can solve the problem. I will give an update about it, but maybe you can take a look at it afterwards @go-jet |
That would give us two almost identical file, but we still wouldn't be sure they are truly identical.
It shouldn't be that complicated. Something like: func (s *selectJsonStatement) AS(alias string) Projection {
if statementType == SelectJsonObjStatementType {
return Func("JSON", s.selectStatementImpl).AS(alias)
}
return s.selectStatementImpl.AS(alias)
} |
|
Small update: I have not been working on it this week, but I think I will finish it next week. |
make the date 0000-01-01 when selecting a time.
json. However, since sqlite does not natively have a base64 function, a custom function is registered inside the main sqlite test.
sqlite requires that the json object is wrapped inside a json function, otherwise sqlite returns a json string, instead of an actual json object.
function for json objects, to deal with different rounding values between returned values from sqlite and json from sqlite, and some tests from postgres.
function was available. Also added json test data inside the test_sample.db.
|
When i made changes to the test data for sqlite (added a json and json_ptr column to the all types database), I think i have to make a PR for that seperate repo? That is the reason the tests are still failing. The PR is not entirely done yet. I still have to clean up the code a bit. And one test is currently commented out since I could not get the same SQL with whitespacing between the actual sql and expected. |
Adds support for selecting json with sqlite. To create tests, I have copied the tests from mysql and added them to sqlite test folder. I made some tweaks to the tests, for example using a different test database for sqlite and making small changes to the sql syntax for the asserts. This PR also changes the qrm package for dealing with returned strings instead of bytes for json values in sqlite.
There are two tests I do not get working, and those are:
TestSelectJson_GroupBy: the returned json from sqlite has different values for items in the amount key. Some sums or averages are slightly different because of rounding errrors, but it does mean the test wont pass. For example:Original from test
{ "CustomerID": 2, "StoreID": 1, "FirstName": "PATRICIA", "LastName": "JOHNSON", "Email": "[email protected]", "AddressID": 6, "Active": "1", "CreateDate": "2006-02-14T22:04:36Z", "LastUpdate": "2019-04-11T18:11:49Z", "Amount": { "Sum": *128.73000000000002*, "Avg": *4.767777777777779*, "Max": 10.99, "Min": 0.99, "Count": 27 }JSON from sqlite json statement:
{ "CustomerID": 2, "StoreID": 1, "FirstName": "PATRICIA", "LastName": "JOHNSON", "Email": "[email protected]", "AddressID": 6, "Active": "1", "CreateDate": "2006-02-14T22:04:36Z", "LastUpdate": "2019-04-11T18:11:49Z", "Amount": { "Sum": *128.73*, "Avg": *4.76777777777778*, "Max": 10.99, "Min": 0.99, "Count": 27 } }The other test that fails needs more work, but i have not been able to solve it. The
TestSelectJsonObj_NestedObjreturns json but the nested json statements are not returned as json by sqlite but as strings containing json (so it is not a nested object but a string). Because of that the json unmarshal function does not work. To make sqlite return a nested object instead of a string, a JSON() statement should be added that wraps the subquery. So the query that fails is:But to make it work, the query should look like this: