[HZ-1119] Nested types support #19954

ivanthescientist · 2021-11-16T13:59:25Z

Adds support for nested fields for Java classes, WIP.

Changes behavior of default OBJECT type to support nested fields.

# Conflicts: # hazelcast-sql/src/main/java/com/hazelcast/sql/impl/calcite/parse/QueryParser.java

# Conflicts: # hazelcast-sql/src/main/java/com/hazelcast/sql/impl/calcite/schema/HazelcastTable.java # hazelcast-sql/src/test/java/com/hazelcast/sql/impl/expression/NestingAndCasingExpressionTest.java

…QUERY

# Conflicts: # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/HazelcastSqlToRelConverter.java # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/opt/OptUtils.java # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/schema/HazelcastCalciteCatalogReader.java # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/schema/HazelcastTable.java # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/validate/HazelcastSqlOperatorTable.java # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/validate/operand/TypedOperandChecker.java # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/validate/operators/typeinference/JsonFunctionOperandTypeInference.java # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/validate/operators/typeinference/json/HazelcastJsonQueryFunction.java # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/validate/operators/typeinference/json/HazelcastJsonValueFunction.java # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/validate/operators/typeinference/json/HazelcastParseJsonFunction.java # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/validate/operators/typeinference/json/JsonFunctionUtil.java # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/validate/types/HazelcastJsonType.java # hazelcast/src/main/java/com/hazelcast/sql/impl/SqlDataSerializerHook.java

# Conflicts: # hazelcast-sql/src/test/java/com/hazelcast/jet/sql/impl/expression/misc/CastFunctionIntegrationTest.java

# Conflicts: # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/connector/SqlConnector.java # hazelcast-sql/src/test/java/com/hazelcast/jet/sql/impl/connector/kafka/SqlJsonTest.java

krzysztofslusarski

First glance over PR

hazelcast/src/main/java/com/hazelcast/jet/impl/util/ReflectionUtils.java

hazelcast/src/main/java/com/hazelcast/sql/impl/client/SqlExecuteMessageTask.java

hazelcast/src/main/java/com/hazelcast/sql/impl/client/SqlFetchMessageTask.java

hazelcast/src/main/java/com/hazelcast/sql/impl/client/SqlExecuteMessageTask.java

krzysztofslusarski · 2022-08-14T19:22:32Z

hazelcast/src/main/java/com/hazelcast/sql/impl/expression/FieldAccessExpression.java

+    }
+
+    private boolean isPrimitive(Class<?> clazz) {
+        return clazz.getPackage().getName().startsWith("java.");


There is a method java.lang.Class#isPrimitive.

Here, String is also primitive. It's a sanity check, it should not normally happen. I think we can convert it to an assert.

It will not work with Map, List etc. We want to skip those too. Bad name for the method though probably.

hazelcast/src/main/java/com/hazelcast/sql/impl/type/converter/RowConverter.java

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/inject/HazelcastObjectUpsertTarget.java

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/schema/TypesUtils.java

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java

This reverts commit f70429c.

mdumandag

On a very superficial review from the client's perspective, it looked okay. I just have one objection, but it is a simple thing, and I will therefore approve this.

But, I would like to spend more time on this PR after BETA release to do a proper review

hazelcast/src/main/java/com/hazelcast/sql/impl/expression/RowValue.java

# Conflicts: # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/schema/TablesStorage.java

…ject-based' into feature/5.1/nested-data-types-object-based

hazelcast/src/main/java/com/hazelcast/jet/impl/util/ReflectionUtils.java

viliam-durina · 2022-08-17T17:56:04Z

hazelcast/src/main/java/com/hazelcast/sql/impl/expression/FieldAccessExpression.java

+    }
+
+    private boolean isPrimitive(Class<?> clazz) {
+        return clazz.getPackage().getName().startsWith("java.");


Here, String is also primitive. It's a sanity check, it should not normally happen. I think we can convert it to an assert.

hz-devops-test · 2022-08-17T19:26:23Z

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file

--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   ClientMapPartitionLostListenerTest.test_mapPartitionLostListener_invoked_fromOtherNode:133->assertProxyExistsEventually:163->HazelcastTestSupport.assertTrueEventually:1338->HazelcastTestSupport.assertTrueEventually:1236 There is no proxy with name 2568f76e-21e5-4de1-8449-2ad16d2918b6 created (yet)
[INFO] 
[ERROR] Tests run: 50737, Failures: 1, Errors: 0, Skipped: 238
[INFO] 
[ERROR] There are test failures.

hz-devops-test · 2022-08-17T19:28:59Z

The job Hazelcast-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file

--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   QueueDestroyTest.checkStatsMapEntryRemovedWhenQueueDestroyed:78 expected:<0> but was:<1>
[INFO] 
[ERROR] Tests run: 50737, Failures: 1, Errors: 0, Skipped: 238
[INFO] 
[ERROR] There are test failures.

ivanthescientist added 30 commits August 9, 2021 14:48

string based impl

1da8f2c

type changes

ea6cf7c

parser fix, type conversion

9f04a57

checkstyle and small other fixes

26a6c2e

license fix

ac6d2dc

fixed dependencies

dc61989

partial JSON type and CAST AS JSON support

64b4f35

Parse JSON support and cast literal json support

fb4a3af

type, test and checkstyle fixes

69a2fe4

fixed enum/converter tests

7776b03

Merge branch 'master' into feature/5.1/basic-json-type-support

7432687

# Conflicts: # hazelcast-sql/src/main/java/com/hazelcast/sql/impl/calcite/parse/QueryParser.java

fixed serialization and tests

0a58635

checkstyle fix

ab8e5f0

temporarily removed support for nested JSON value

65393e6

json-query extended syntax support

c6adb87

added type test, fixed inserts for json

f5ee2f2

Merge branch 'master' into feature/5.1/basic-json-type-support

ddec064

small fixes

181ddbd

Merge branch 'master' into feature/5.1/basic-json-type-support

ffba762

# Conflicts: # hazelcast-sql/src/main/java/com/hazelcast/sql/impl/calcite/schema/HazelcastTable.java # hazelcast-sql/src/test/java/com/hazelcast/sql/impl/expression/NestingAndCasingExpressionTest.java

merge fix

d38f379

pom fix

557839e

basic JSON_VALUE function support, symbol serialization fix for JSON_…

122a10c

…QUERY

post-merge fixes

d24b3f3

extended JsonValue syntax support, refactors

658f4a5

removed json type resolver

e17ca91

Merge branch 'master' into feature/5.1/basic-json-type-support

4ed8e60

# Conflicts: # hazelcast-sql/src/test/java/com/hazelcast/jet/sql/impl/expression/misc/CastFunctionIntegrationTest.java

json type support for kafka connector

0e9df9b

Merge branch 'master' into feature/5.1/basic-json-type-support

884f057

# Conflicts: # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/connector/SqlConnector.java # hazelcast-sql/src/test/java/com/hazelcast/jet/sql/impl/connector/kafka/SqlJsonTest.java

fixed tests

3bfd4b7

hazelcast deleted a comment from hz-devops-test Aug 16, 2022

user-defined-types-enabled config option

f70429c

hazelcast deleted a comment from hz-devops-test Aug 16, 2022

krzysztofslusarski reviewed Aug 16, 2022

View reviewed changes

hazelcast deleted a comment from hz-devops-test Aug 17, 2022

viliam-durina added 7 commits August 17, 2022 10:28

Merge branch 'master' into feature/5.1/nested-data-types-object-based

a196d2f

Handle missing type used

5d2a12e

Fail in TO_ROW, if there's a cycle

ebc817a

Run basic test on member and client instances

ecbd837

Revert "user-defined-types-enabled config option"

a37bc37

This reverts commit f70429c.

Add a property to enable custom types

2740d90

Fix return type of TO_ROW

feb0bd8

mdumandag approved these changes Aug 17, 2022

View reviewed changes

hazelcast/src/main/java/com/hazelcast/sql/impl/expression/RowValue.java Show resolved Hide resolved

ivanthescientist and others added 6 commits August 17, 2022 20:32

removed typesStorage

ad25bf6

Minor changes

ac731ca

Merge branch 'master' into feature/5.1/nested-data-types-object-based

597e891

# Conflicts: # hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/schema/TablesStorage.java

merge fix

7db90ed

Merge remote-tracking branch 'origin/feature/5.1/nested-data-types-ob…

3081d42

…ject-based' into feature/5.1/nested-data-types-object-based

Minor changes2

6265c52

hazelcast deleted a comment from hz-devops-test Aug 17, 2022

viliam-durina approved these changes Aug 17, 2022

View reviewed changes

Address Krzysztof's review

4df29b6

hazelcast deleted a comment from hz-devops-test Aug 17, 2022

krzysztofslusarski approved these changes Aug 18, 2022

View reviewed changes

ivanthescientist merged commit 67a8b58 into hazelcast:master Aug 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HZ-1119] Nested types support #19954

[HZ-1119] Nested types support #19954

ivanthescientist commented Nov 16, 2021 •

edited

Loading

krzysztofslusarski left a comment

krzysztofslusarski Aug 14, 2022

viliam-durina Aug 17, 2022

ivanthescientist Aug 17, 2022 •

edited

Loading

mdumandag left a comment

viliam-durina Aug 17, 2022

hz-devops-test commented Aug 17, 2022

hz-devops-test commented Aug 17, 2022

[HZ-1119] Nested types support #19954

[HZ-1119] Nested types support #19954

Conversation

ivanthescientist commented Nov 16, 2021 • edited Loading

krzysztofslusarski left a comment

Choose a reason for hiding this comment

krzysztofslusarski Aug 14, 2022

Choose a reason for hiding this comment

viliam-durina Aug 17, 2022

Choose a reason for hiding this comment

ivanthescientist Aug 17, 2022 • edited Loading

Choose a reason for hiding this comment

mdumandag left a comment

Choose a reason for hiding this comment

viliam-durina Aug 17, 2022

Choose a reason for hiding this comment

hz-devops-test commented Aug 17, 2022

hz-devops-test commented Aug 17, 2022

ivanthescientist commented Nov 16, 2021 •

edited

Loading

ivanthescientist Aug 17, 2022 •

edited

Loading