Introduce Object Type handling in SQL [HZ-2312] #24356

TomaszGaweda · 2023-04-24T17:33:04Z

Connector as for now won't define isStreaming method, but this will be a property of HazelcastTable.
Connector should decide on streaming = true|false based on the objectType
SqlConnector was refactored to return DTO of Mapping instead of most of fields as arguments.

Note:
This PR replaces:

create mapping myMongo 
type MongoStream (...)

with

create mapping myMongo
type Mongo object type ChangeStream

Mongo is a new beta feature in 5.3, so it's not a separate thing for RNs, but docs must be updated accordingly.

Jira: HZ-2312
Fixes: #24302

Breaking changes (list specific methods/types/messages):

API - SqlConnector

Checklist:

Labels (Team:, Type:, Source:, Module:) and Milestone set
Label Add to Release Notes or Not Release Notes content set
Request reviewers if possible
[-] Send backports/forwardports if fix needs to be applied to past/future releases
New public APIs have @Nonnull/@Nullable annotations
New public APIs have @since tags in Javadoc

…Type

TomaszGaweda · 2023-04-24T19:11:22Z

run-lab-run

frant-hartm

Approving.

@Fly-Style @k-jamroz Please check the getAllDataConnectionNameWithTypes method as it returns row as Object[] inside which there is an ArrrayList, with type JSON.

No idea if that is correct or not.

hazelcast-sql/src/main/java/com/hazelcast/sql/impl/schema/view/View.java

Fly-Style · 2023-04-26T09:48:27Z

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/connector/virtual/ViewTable.java

-    public ViewTable(String schemaName, String viewName, String viewQuery, TableStatistics statistics) {
-        super(schemaName, viewName, null, statistics);
+    public ViewTable(String schemaName, String viewName, String viewQuery, TableStatistics statistics, boolean streaming) {
+        super(schemaName, viewName, null, statistics, "View", streaming);


Overall, I am coming to the conclusion we can avoid defining 'streaming' view here and do it later based on OptUtils.isUnbounded(viewRel) in initFields method. WDYT?

So basically always pass e.g. false and set "real" value in initFields? I'm not that familiar with SQL yet, but if initFields is mandatory, then it makes sense

Fly-Style · 2023-04-26T09:50:16Z

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

-                        .getAllDataConnectionEntries(dataConnectionService, dataConnectionCatalog.getDataConnectionStorage())
-                        .stream()
-                        .map(DataConnectionCatalogEntry::name);
+                        .getAllDataConnectionNameWithTypes(dataConnectionService)


You changed SHOW DATA CONNECTIONS behavior, is it intended?

Yes. Correct me if I'm wrong, but semantics will be the same - all the same connections will be shown. The only difference is that for SHOW DATA CONNECTIONS we add additional column to provide user an information about supported object types. Idea is from this message: https://hazelcast.slack.com/archives/CJZ745RRT/p1681908716511929?thread_ts=1681849114.133969&cid=CJZ745RRT

this causes #24415, OBJECT TYPE without CONNECTION TYPE does not make much sense.
In slack thread we dissussed adding it to SHOW RESOURCES not to SHOW DATA CONNECTIONS

ivanthescientist · 2023-04-26T09:11:31Z

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

+                plan.getShowTarget() == ShowStatementTarget.DATACONNECTIONS
+                ?  new SqlRowMetadata(asList(
+                        new SqlColumnMetadata("name", VARCHAR, false),
+                        new SqlColumnMetadata("types", JSON, false)


Why JSON specifically? JSON is just a wrapper type anyway, its not like its going to provide much utility on clients or is there actually a benefit to it?

It will be parseable by the JDBC client

this causes #24415, OBJECT TYPE without CONNECTION TYPE does not make much sense

ivanthescientist · 2023-04-26T09:37:51Z

hazelcast-sql/src/main/java/com/hazelcast/sql/impl/schema/view/View.java

@@ -82,6 +88,7 @@ public void writeData(ObjectDataOutput out) throws IOException {
        }
        SerializationUtil.writeList(viewColumnNames, out);
        SerializationUtil.writeList(viewColumnTypes, out);
+        out.writeBoolean(streaming);


Do we need a version guard here actually? E.g. in.getVersion().isGreaterOrEqual(Versions.V5_3).. And it will most likely be a good idea to test this in a rolling upgrade setup just to be sure I guess (probably enough to just create some schema objects in 5.2 node and join a 5.3, verify that it has the schema objects and then drop the 5.2 member and verify that 5.3's schema is working as expected).

In a rolling upgrade these schema objects will still be transferred because they're in a regular IMap. We might need some mechanism post 5.2 -> 5.3 upgrade for handling that.

Actually this field is no longer used if I initialize ViewTable's streaming property based on OptUtils.isUnbounded. I will remove it.

the field was not removed from serialized form

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

hz-devops-test · 2023-04-26T10:55:11Z

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

hz-devops-test · 2023-04-26T11:01:17Z

The job Hazelcast-pr-compiler 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

…atures/5.3/objectType

k-jamroz · 2023-04-27T15:08:57Z

hazelcast-sql/src/main/java/com/hazelcast/sql/impl/schema/BadTable.java

@@ -31,7 +31,7 @@ public final class BadTable extends Table {
    private final Throwable cause;

    public BadTable(String schemaName, String sqlName, Throwable cause) {
-        super(schemaName, sqlName, Collections.emptyList(), new ConstantTableStatistics(0));
+        super(schemaName, sqlName, Collections.emptyList(), new ConstantTableStatistics(0), "Bad", false);


isStreaming() should throw, we could pass original object type of bad mapping

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

k-jamroz · 2023-04-27T16:06:01Z

hazelcast/src/main/java/com/hazelcast/dataconnection/HazelcastDataConnection.java

+    @Nonnull
+    @Override
+    public Collection<String> resourceTypes() {
+        return Collections.singleton("IMap");


should use constants so it is defined in single place

Follow-up to #24356 Changes: - Remove View.streaming field completely, Ensure that isStreaming for view is determined - Use original object type for bad table - Update SHOW DATA CONNECTIONS description in TDD - Use constants for object type - Refactor SqlMappingContext and simplify SqlConnector.resolveAndValidateFields - Resource type is case-insensitive

TomaszGaweda added 2 commits April 24, 2023 19:27

Move streaming info to Table, refactor SqlConnector to include object…

250d512

…Type

Fixed streaming for views

fde2e79

TomaszGaweda added Type: Enhancement Source: Internal PR or issue was opened by an employee Team: SQL Not Release Notes content Module: SQL and removed Not Release Notes content labels Apr 24, 2023

TomaszGaweda self-assigned this Apr 24, 2023

TomaszGaweda added the SQL-only Use when changes are only in hazelcast-sql module label Apr 24, 2023

Fixed streaming for views

2e55859

hazelcast deleted a comment from hz-devops-test Apr 24, 2023

TomaszGaweda added 3 commits April 24, 2023 20:20

Default object type

dc2e96a

Adjusted SHOW DATA CONNECTIONS

d26552e

Adjust test

2198375

hazelcast deleted a comment from hz-devops-test Apr 24, 2023

Checkstyle

5ec2983

TomaszGaweda added this to the 5.3.0 milestone Apr 24, 2023

TomaszGaweda added Team: Integration and removed SQL-only Use when changes are only in hazelcast-sql module labels Apr 24, 2023

TomaszGaweda changed the title ~~WIP ObjectType~~ WIP ObjectType [HZ-2312] Apr 24, 2023

hazelcast deleted a comment from hz-devops-test Apr 24, 2023

JavaDoc, better encapsulation

79d38f3

frant-hartm approved these changes Apr 25, 2023

View reviewed changes

Fly-Style reviewed Apr 26, 2023

View reviewed changes

hazelcast-sql/src/main/java/com/hazelcast/sql/impl/schema/view/View.java Outdated Show resolved Hide resolved

Fly-Style reviewed Apr 26, 2023

View reviewed changes

ivanthescientist reviewed Apr 26, 2023

View reviewed changes

Fly-Style reviewed Apr 26, 2023

View reviewed changes

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java Outdated Show resolved Hide resolved

CR comments

bf9c2cc

TomaszGaweda requested review from ivanthescientist and Fly-Style April 26, 2023 10:50

TomaszGaweda added 3 commits April 26, 2023 13:22

Somehow this wasn't committed

ffe8e1c

Fixes

1dd7ce8

Merge remote-tracking branch 'origin/features/5.3/objectType' into fe…

03afb6f

…atures/5.3/objectType

hazelcast deleted a comment from hz-devops-test Apr 26, 2023

TomaszGaweda removed the request for review from k-jamroz April 26, 2023 13:21

Fly-Style approved these changes Apr 27, 2023

View reviewed changes

ivanthescientist approved these changes Apr 27, 2023

View reviewed changes

Merge branch 'master' into features/5.3/objectType

6b3bf7f

frant-hartm enabled auto-merge (squash) April 27, 2023 12:26

frant-hartm merged commit 64d6863 into hazelcast:master Apr 27, 2023
8 checks passed

k-jamroz reviewed Apr 27, 2023

View reviewed changes

k-jamroz mentioned this pull request Apr 27, 2023

Fix some issues with ObjectType change [HZ-2312] #24406

Merged

6 tasks

k-jamroz reviewed Apr 27, 2023

View reviewed changes

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/schema/TableResolverImpl.java Show resolved Hide resolved

k-jamroz reviewed Apr 27, 2023

View reviewed changes

k-jamroz mentioned this pull request Apr 28, 2023

SHOW DATA CONNECTIONS prints 'java.util.ArrayList cannot be cast to com.hazelcast.core.HazelcastJsonValue' [HZ-2384] #24415

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce Object Type handling in SQL [HZ-2312] #24356

Introduce Object Type handling in SQL [HZ-2312] #24356

TomaszGaweda commented Apr 24, 2023 •

edited

TomaszGaweda commented Apr 24, 2023

frant-hartm left a comment

Fly-Style Apr 26, 2023

TomaszGaweda Apr 26, 2023

TomaszGaweda Apr 26, 2023

Fly-Style Apr 26, 2023

TomaszGaweda Apr 26, 2023

k-jamroz Apr 28, 2023 •

edited

ivanthescientist Apr 26, 2023

TomaszGaweda Apr 26, 2023

k-jamroz Apr 28, 2023

ivanthescientist Apr 26, 2023

TomaszGaweda Apr 26, 2023

TomaszGaweda Apr 26, 2023

k-jamroz Apr 27, 2023

k-jamroz Apr 27, 2023

hz-devops-test commented Apr 26, 2023

hz-devops-test commented Apr 26, 2023

k-jamroz Apr 27, 2023

k-jamroz Apr 27, 2023

k-jamroz Apr 27, 2023

k-jamroz Apr 27, 2023

Introduce Object Type handling in SQL [HZ-2312] #24356

Introduce Object Type handling in SQL [HZ-2312] #24356

Conversation

TomaszGaweda commented Apr 24, 2023 • edited

TomaszGaweda commented Apr 24, 2023

frant-hartm 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

k-jamroz Apr 28, 2023 • edited

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

hz-devops-test commented Apr 26, 2023

hz-devops-test commented Apr 26, 2023

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

TomaszGaweda commented Apr 24, 2023 •

edited

k-jamroz Apr 28, 2023 •

edited