Json processing in postgres notation #1810

lazarevnik · 2019-03-15T14:34:39Z

Hello!
Here I've implemented :

JSON data type processing based on fasterXML/jackson library
Functions to work with documents
Special operations for querying with Postgres syntax

Also here are some queries shows how it works.

Unfortunately, I've got some problems with memory and network tests.
I'll glad to discuss their decision.

…o json-postgres

katzyn

Thank you for working on this feature.

I didn't check all changes yet, so it's not a complete review.

H2 should work without any third-party libraries. JSON data type may not work without them, but all other features should not require them.

Also it's not a good idea to allow usage of third-party data types in getObject() / setObject() / etc., Jakson may be removed and replaced with something else. Perhaps JSON should be mapped to String or byte[]. If you need such support, reimplement it with a reflection.

katzyn · 2019-03-15T14:49:53Z

h2/src/main/org/h2/command/Parser.java

+                    throw getSyntaxError();
+                }
+                int len = args.length;
+                Value[] arr = (Value[]) Array.newInstance(Value.class, len);


new Value[len]

katzyn · 2019-03-15T14:50:44Z

h2/src/main/org/h2/command/Parser.java

+                String[] args;
+                if(param.startsWith("{") && param.endsWith("}")) {
+                    param = param.substring(1, param.length() - 1);
+                    args = param.replaceAll(" ", "").split(",");


I don't think that Parser is a good place for such logic.

katzyn · 2019-03-15T14:53:14Z

h2/src/main/org/h2/command/Parser.java

+                int len = args.length;
+                Value[] arr = (Value[]) Array.newInstance(Value.class, len);
+                for (int i = 0; i < len; i++) {
+                    Value v = StringUtils.isNumber(args[i]) ? ValueInt.get(new Integer(args[i])) : ValueString.get(args[i]);


Do not use new Integer(String), it is deprecated.

katzyn · 2019-03-15T14:54:52Z

h2/src/main/org/h2/expression/function/Function.java

@@ -1759,6 +1788,212 @@ protected Value getValueWithArgs(Session session, Expression[] args) {
            String msgText = v1.getString();
            throw DbException.fromUser(sqlState, msgText);
        }
+        case JSON_FIELD: {
+            if(v0.getValueType() == Value.JSON) {
+                JsonNode json = ((ValueJson) v0).getObject();


All such code with third-party classes should be isolated into separate methods or classes.

katzyn · 2019-03-15T14:55:54Z

h2/src/main/org/h2/value/DataType.java

@@ -1112,6 +1151,8 @@ public static int getTypeFromClass(Class <?> x) {
            return Value.TIMESTAMP;
        } else if (LocalDateTimeUtils.OFFSET_DATE_TIME == x || LocalDateTimeUtils.INSTANT == x) {
            return Value.TIMESTAMP_TZ;
+        } else if (com.fasterxml.jackson.databind.JsonNode.class == x) {


Do not create hard run-time dependencies of third-party libraries.

h2/src/main/org/h2/value/Value.java

h2/src/main/org/h2/value/ValueJson.java

katzyn · 2019-03-15T15:00:08Z

h2/src/main/org/h2/value/ValueJson.java

+    @Override
+    public int compareTypeSafe(Value v, CompareMode mode) {
+        // TODO Auto-generated method stub
+        return 0;


This method should be implemented.

katzyn · 2019-03-15T15:00:40Z

h2/src/test/org/h2/test/scripts/datatypes/json.sql

@@ -0,0 +1,50 @@
+SELECT CAST('{"tag1":"simple string"}' AS JSON);


Missing copyright header.

h2/src/main/org/h2/value/Transfer.java

katzyn · 2019-03-15T15:20:50Z

h2/src/main/org/h2/value/ValueJson.java

+    private static final ObjectMapper mapper = new ObjectMapper();
+
+    ValueJson(String str) throws IOException {
+        int memFirst = Utils.getMemoryUsed();


Do not use this method in the main code, it's not reliable.

It's not entirely clear how to evaluate various json objects.
Here used string length, but it may work not always.

katzyn · 2019-03-15T15:21:37Z

h2/src/main/org/h2/value/ValueJson.java

+            this.json=mapper.createObjectNode();
+            this.string = this.json.toString();
+        }
+        this.string = str.replaceAll("\n", "");


Perhaps JSON should be fully normalized into some canonical form.

h2/src/main/org/h2/value/ValueJson.java

katzyn · 2019-03-15T15:30:12Z

h2/src/test/org/h2/test/scripts/TestScript.java

@@ -427,7 +427,7 @@ private void process(String sql, boolean allowReconnect) throws Exception {
        if (statements != null) {
            statements.add(sql);
        }
-        if (sql.indexOf('?') == -1) {
+        if (sql.indexOf('?') == -1 || sql.indexOf("?'") != -1 || sql.indexOf("?|") != -1 || sql.indexOf("?&") != -1) {


Such check is not reliable. Characters next to ? should be tested instead. Not very critical for now, but at least a TODO comment should be placed here to indicate a problem.

Current check used to exclude collision between JSON operators and parameter token because it's not obvious how this can be resolved at the parser level.

katzyn · 2019-03-15T15:37:59Z

h2/src/main/org/h2/command/Parser.java

+    /**
+     * The PostgreSQL token "||"
+     */
+    private static final int JSON_CONCAT = JSON_EXISTS_ALL + 1;


How it is distinguished from a string || concatenation and why these constants are not consistent with TOKENS array?

katzyn · 2019-03-16T01:59:28Z

h2/src/main/org/h2/expression/function/Function.java

+        addFunction("JSON_EXISTS_ANY", JSON_EXISTS_ANY, 2, Value.BOOLEAN);
+        addFunction("JSON_EXISTS_ALL", JSON_EXISTS_ALL, 2, Value.BOOLEAN);
+        addFunction("JSON_CONCAT", JSON_CONCAT, 2, Value.JSON);
+        addFunction("JSON_DELETE_FIELD", JSON_DELETE_FIELD, 2, Value.JSON);


Where all these functions came from? Neither SQL Standard nor PostgreSQL documentation have them with an exception for JSON_EXISTS that is described in the standard (but I'm not sure that it is compatible with your implementation).

If you need them to support some operations, it's better to use own class similar to BinaryOperation instead. Operations can be mapped to functions only when a similar function exists.

Yes, now these functions are used for the work of operators. With the development of the functionality, it is planned to add new ones, but I'll try to correlate names with the standard or the PostgreSQL dialect.

katzyn · 2019-03-16T02:03:23Z

JSON support was standardized in SQL:2016, but PostgreSQL is not compatible with the standard. I wonder how many complexity these weird PostgreSQL-only operations create for us when we decide to support standard features.

#> and #>> may create problems in Oracle and MS SQL compatibility modes (they aren't tested well, so no failures on Travis doesn't mean that everything works as expected). Oracle has standard JSON features, BTW. I'm not sure about all these ->>, @>, ?|, etc. @grandinj, may be you have some opinion?

grandinj · 2019-03-16T19:06:29Z

@lazarevnik Thank you very much for this!

@katzyn I don't really like adding these operators, I'd prefer functions, they are much easier to extend and much less likely to cause trouble with parsing.

@lazarevnik Are you deliberately aiming at Postgresql compatibilty?

lazarevnik · 2019-03-18T15:11:43Z

@katzyn Thank you for the review, it is very helpful.
@grandinj Yes, I work with the implementation of this particular dialect for further using it in Apache Ignite to caching. It is also planned to develop functionality and supported dialects.

katzyn · 2019-03-18T15:21:27Z

There is a SQL Standard. Why you need non-standard syntax for Ignite? Sorry, I don't understand it. H2 is not a PostgreSQL emulator. Many databases have own legacy syntax constructions in different areas, but usually they begin to support standard features too.

lazarevnik · 2019-03-18T15:52:22Z

I want to work towards caching with read-/write-through. Ignite allows you to use SQL queries for this on a par to ordinary cache mechanism. So, to work with not abstract standard, but with concrete RDBMS, I used PostgreSQL syntax and operators.

katzyn · 2019-03-18T16:15:08Z

It means that you don't have a real reason, because you're not restricted to that proprietary syntax. Oracle uses the standard, MySQL partially compatible with the standard, there were some attempts to made PostgreSQL compatible with the standard in 2017, but it looks like they aren't yet merged. I don't understand why you didn't discuss your addition with H2 community earlier, but what's done is done.

New data type is fine.
Strong dependency on third-party libraries that were introduced here is not acceptable. H2 must work without them (with possible loss of JSON support).
Non-standard operators are bad idea, we may accept them, but for now I don't see a reason to have them. They may create more problems than benefits for us.

Could you extract ValueJson and all related changes in Value, storage backends, etc. into a separate pull request? Without all these functions and operators? I think that we can merge the data type by itself, clean up its implementation if necessary, and continue our discussion about functions and operators.

lazarevnik · 2019-03-18T16:29:34Z

Yes, I'll do it in near future

lazarevnik · 2019-03-22T14:48:13Z

Here new pull request.

fedorchuck · 2020-04-15T08:37:50Z

h2/pom.xml

+    <dependency>
+      <groupId>com.fasterxml.jackson.core</groupId>
+      <artifactId>jackson-annotations</artifactId>
+      <version>2.9.8</version>


Q: probably it will be good to change scope to provided for this dependencies, to avoid dependency conflicts

Suggested change

<version>2.9.8</version>

<version>2.9.8</version>

<scope>provided</scope>

fedorchuck · 2020-04-15T08:42:06Z

h2/src/main/org/h2/value/DataType.java

+        Class<?> j;
+        try {
+            j = JdbcUtils.loadUserClass(JSON_CLASS_NAME);
+        } catch (Exception e) {


Please catch exceptions which you expect but not so big as this one

lazarevnik added 29 commits September 4, 2018 17:34

Adding a new data type based on jackson library

ab0db21

Parser preparing

e005657

Json functions processing with plugs

18507e2

update jackson library

c3bbbfd

function working update

6ae55a7

parser fixing

93c6921

json storage fix

861fd93

merge conflicts resolved

9362c3a

fix ValueJson Type and ValueType

df96665

DataType spellcheck

7e6a759

JSON type fix in TypeInfo

a9146fa

storage layer JSON type fix

28d3233

JSON type on transfer layer

260e8f8

Json ValueType usage in functions

c5dabe2

Json get array field function fix

4da4170

Parser of JSON functions fixed to reusable

a0d1c52

JSON fix get_field_path functions

f3f4040

Adding jackson library to classpath

1460cb7

spellcheck

07df6f8

json substruction instead of JSON_DELETE_FIELD

f342c8f

Json: unimplemented method JSON_DELETE_PATH hidden

4906164

resolve JSON and String concat conflict

af558b8

useless checks removed

0f8ff35

fix JSON comparing

7c77e06

temporary fix of sintax collision with '<@'

8f41beb

careful storage operation with JSON

6457cf7

fix some tests

59d86a2

JSON tests (problems running in network mode)

3d6a13a

Merge branch 'master' of https://github.com/h2database/h2database int…

d876292

…o json-postgres

katzyn reviewed Mar 15, 2019

View reviewed changes

h2/src/main/org/h2/value/Transfer.java Show resolved Hide resolved

katzyn reviewed Mar 15, 2019

View reviewed changes

katzyn reviewed Mar 16, 2019

View reviewed changes

lazarevnik added 2 commits March 18, 2019 17:23

fix some ValueJson basic functions

004d312

Memory usage of ValueJson changed

cbd2a78

isolate fasterxml usage

9712cbd

fedorchuck reviewed Apr 15, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Json processing in postgres notation #1810

Json processing in postgres notation #1810

lazarevnik commented Mar 15, 2019

katzyn left a comment

katzyn Mar 15, 2019

katzyn Mar 15, 2019

katzyn Mar 15, 2019

katzyn Mar 15, 2019

katzyn Mar 15, 2019

katzyn Mar 15, 2019

lazarevnik Mar 18, 2019

katzyn Mar 15, 2019

katzyn Mar 15, 2019

lazarevnik Mar 18, 2019

katzyn Mar 15, 2019

katzyn Mar 15, 2019

lazarevnik Mar 18, 2019

katzyn Mar 15, 2019

katzyn Mar 16, 2019

lazarevnik Mar 18, 2019

katzyn commented Mar 16, 2019

grandinj commented Mar 16, 2019

lazarevnik commented Mar 18, 2019

katzyn commented Mar 18, 2019

lazarevnik commented Mar 18, 2019

katzyn commented Mar 18, 2019

lazarevnik commented Mar 18, 2019

lazarevnik commented Mar 22, 2019

fedorchuck Apr 15, 2020

fedorchuck Apr 15, 2020

		@@ -0,0 +1,50 @@
		SELECT CAST('{"tag1":"simple string"}' AS JSON);

	<version>2.9.8</version>
	<version>2.9.8</version>
	<scope>provided</scope>

Json processing in postgres notation #1810

Are you sure you want to change the base?

Json processing in postgres notation #1810

Conversation

lazarevnik commented Mar 15, 2019

katzyn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katzyn commented Mar 16, 2019

grandinj commented Mar 16, 2019

lazarevnik commented Mar 18, 2019

katzyn commented Mar 18, 2019

lazarevnik commented Mar 18, 2019

katzyn commented Mar 18, 2019

lazarevnik commented Mar 18, 2019

lazarevnik commented Mar 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment