Persistence Component - Big query support #1904

prasar-ashutosh · 2023-06-08T03:34:26Z

What type of PR is this?

Improvement

What does this PR do / why is it needed ?

Big Query Support
Interface update to handle Big Query
Support for Derived dataset for all milestoning schemes
SQL & E2E Tests for Big Query

Which issue(s) this PR fixes:

Fixes #

Other notes for reviewers:

Does this PR introduce a user-facing change?

NO

linux-foundation-easycla · 2023-06-08T03:34:31Z

❌ - login: @prasar-ashutosh / The commit (110a3ee, c732cc4, 2ad0541, 12b309e, a50db2b, 35c1955, d5ca20a, 21f3dd8, 6a23b0a, 6320261, 57ecb51, c90acfa, 31e6d5b, 28da86e, e68b762, 9345f66, 2782f10, 54a2b34) is not authorized under a signed CLA. "Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.
❌ - login: @shubham43 / The commit (a459ab5, dd14366, c9ca920, 5c9979d, 18c2ef4, 508370f, bcd4c60, d2c13f1, 68637bd, ebf95c3, 1610805, 0ffd1a3) is not authorized under a signed CLA. "Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

github-actions · 2023-06-08T03:36:04Z

Test Results

    474 files     474 suites 47m 5s ⏱️
  9 848 tests   9 517 ✔️ 331 💤 0 ❌
11 710 runs 11 313 ✔️ 397 💤 0 ❌

Results for commit ae4b796.

♻️ This comment has been updated with latest results.

…hemes. Added tests for Big Query

TheJuanAndOnly99 · 2023-06-26T16:25:27Z

/easycla

…y_support # Conflicts: # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/pom.xml # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/main/java/org/finos/legend/engine/persistence/components/relational/bigquery/BigQuerySink.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/main/java/org/finos/legend/engine/persistence/components/relational/bigquery/executor/BigQueryExecutor.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/main/java/org/finos/legend/engine/persistence/components/relational/bigquery/executor/BigQueryHelper.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/main/java/org/finos/legend/engine/persistence/components/relational/bigquery/executor/BigQueryTransactionManager.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/main/java/org/finos/legend/engine/persistence/components/relational/bigquery/optmizer/StringCaseOptimizer.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/main/java/org/finos/legend/engine/persistence/components/relational/bigquery/sql/visitor/DeleteVisitor.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/main/java/org/finos/legend/engine/persistence/components/relational/bigquery/sql/visitor/FieldVisitor.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/main/java/org/finos/legend/engine/persistence/components/relational/bigquery/sqldom/schemaops/statements/AlterTable.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/test/java/org/finos/legend/engine/persistence/components/BaseTestUtils.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/test/java/org/finos/legend/engine/persistence/components/e2e/BigQueryEndToEndTest.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/test/java/org/finos/legend/engine/persistence/components/ingestmode/BigQueryTestArtifacts.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/test/java/org/finos/legend/engine/persistence/components/ingestmode/BitemporalDeltaSourceSpecifiesFromTest.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/test/java/org/finos/legend/engine/persistence/components/ingestmode/NontemporalDeltaTest.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/test/java/org/finos/legend/engine/persistence/components/logicalplan/operations/AlterTest.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/test/resources/expected/append/data_pass2.csv # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/src/test/resources/expected/nontemporal_delta/data_pass2.csv # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-core/src/main/java/org/finos/legend/engine/persistence/components/relational/RelationalSink.java # legend-engine-xt-persistence-component/legend-engine-xt-persistence-component-relational-core/src/main/java/org/finos/legend/engine/persistence/components/relational/sqldom/common/Clause.java

kumuwu · 2023-06-30T08:47:29Z

.../finos/legend/engine/persistence/components/relational/bigquery/executor/BigQueryHelper.java

+
+
+ @Override
+ public void close()


Is this empty block intentional?

Yes, intentional, JDBC Helper actually has a JDBC connection that needs to be closed explicitly, but Big Query doesn't need to close anything, that's why this is empty.

Wont be better to have closeTransactionManager as private, and inside this close call that method? That way, you have a single close that cleans all resources, and in the future other resources might be added too...

Changed as suggested, But keeping closeTransactionManager as public as it's inherited from RelationalExecutionHelper.

…d method params

…y_support

rafaelbey · 2023-07-03T18:10:26Z

...in/java/org/finos/legend/engine/persistence/components/relational/bigquery/BigQuerySink.java

+ implicitDataTypeMapping.put(DataType.INTEGER, new HashSet<>(Arrays.asList(DataType.INT, DataType.BIGINT, DataType.TINYINT, DataType.SMALLINT, DataType.INT64)));
+ implicitDataTypeMapping.put(DataType.NUMERIC, new HashSet<>(Arrays.asList(DataType.NUMERIC, DataType.NUMBER, DataType.DECIMAL, DataType.INT, DataType.INTEGER, DataType.BIGINT, DataType.TINYINT, DataType.SMALLINT, DataType.INT64)));
+ implicitDataTypeMapping.put(DataType.FLOAT, new HashSet<>(Arrays.asList(DataType.REAL, DataType.DOUBLE, DataType.FLOAT64, DataType.INT, DataType.INTEGER, DataType.BIGINT, DataType.TINYINT, DataType.SMALLINT, DataType.INT64, DataType.NUMBER, DataType.NUMERIC, DataType.DECIMAL)));
+ implicitDataTypeMapping.put(DataType.STRING, new HashSet<>(Arrays.asList(DataType.STRING, DataType.CHAR, DataType.CHARACTER, DataType.VARCHAR, DataType.LONGNVARCHAR, DataType.LONGTEXT, DataType.TEXT)));


These HashSet should be wrappd on unmodifiable to actually make the IMPLICIT_DATA_TYPE_MAPPING fully unmodifiable.

Thanks, Changed as suggested

rafaelbey · 2023-07-03T18:10:42Z

...in/java/org/finos/legend/engine/persistence/components/relational/bigquery/BigQuerySink.java

+ explicitDataTypeMapping.put(DataType.NUMBER, new HashSet<>(Arrays.asList(DataType.REAL, DataType.FLOAT, DataType.DOUBLE, DataType.FLOAT64)));
+ explicitDataTypeMapping.put(DataType.NUMERIC, new HashSet<>(Arrays.asList(DataType.REAL, DataType.FLOAT, DataType.DOUBLE, DataType.FLOAT64)));
+ explicitDataTypeMapping.put(DataType.DECIMAL, new HashSet<>(Arrays.asList(DataType.REAL, DataType.FLOAT, DataType.DOUBLE, DataType.FLOAT64)));
+ EXPLICIT_DATA_TYPE_MAPPING = Collections.unmodifiableMap(explicitDataTypeMapping);


Same as above - These HashSet should be wrappd on unmodifiable to actually make the IMPLICIT_DATA_TYPE_MAPPING fully unmodifiable.

Thanks, Changed as suggested

rafaelbey · 2023-07-03T18:13:53Z

.../finos/legend/engine/persistence/components/relational/bigquery/executor/BigQueryHelper.java

+
+
+ @Override
+ public void close()


Wont be better to have closeTransactionManager as private, and inside this close call that method? That way, you have a single close that cleans all resources, and in the future other resources might be added too...

rafaelbey · 2023-07-03T18:18:33Z

...-xt-persistence-component/legend-engine-xt-persistence-component-relational-bigquery/pom.xml

+ <dependency>
+ <groupId>com.google.cloud</groupId>
+ <artifactId>google-cloud-bigquery</artifactId>
+ <scope>provided</scope>


How these provided dependencies will work?

Since it's a library, the user of the library is supposed to provide the dependencies.

...os/legend/engine/persistence/components/relational/bigquery/optmizer/LowerCaseOptimizer.java

...os/legend/engine/persistence/components/relational/bigquery/optmizer/UpperCaseOptimizer.java

rafaelbey · 2023-07-03T18:26:32Z

...os/legend/engine/persistence/components/relational/bigquery/sql/BigQueryDataTypeMapping.java

+ case TINYINT:
+ case SMALLINT:
+ case INT64:
+ dataType = new Int64();


What is the rational to fit everything on 64 bit? Seems as something that will lead to incorrect data.

Im not sure how these mappings are used, but if we allow 64 bit on some places that then we try to insert into the database into TinyInt, we might end with bit overflows, and we might not know the value on the DB will be wrong.

Thoughts?

This mapping is for mapping logical data types to BigQuery data types.
In Big Query, INT, SMALLINT, INTEGER, BIGINT, TINYINT, and BYTEINT are aliases for INT64.
So we have mapped all of them to INT64.
https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#numeric_types

rafaelbey · 2023-07-03T18:27:52Z

...persistence/components/relational/bigquery/sql/BigQueryDataTypeToLogicalDataTypeMapping.java

+ return FieldType.builder().dataType(DataType.NUMERIC).length(columnSize).scale(decimalDigits).build();
+ case FLOAT64:
+ case FLOAT:
+ return FieldType.builder().dataType(DataType.FLOAT).build();


On the same line as prev comment on INT64, FLOAT and FLAOT 64 are two different types that have different ranges of values they can store.

BigQuery only supports FLOAT64, so we have mapped both these logical types to FLOAT64

...ngine/persistence/components/relational/bigquery/sql/visitor/BatchStartTimestampVisitor.java

...inos/legend/engine/persistence/components/relational/bigquery/sql/visitor/DeleteVisitor.java

prasar-ashutosh requested a review from a team as a code owner June 8, 2023 03:34

prasar-ashutosh marked this pull request as draft June 8, 2023 03:37

prasar-ashutosh and others added 12 commits June 8, 2023 11:49

Initial Code for big query module

110a3ee

Added different Visitors for BigQuerySink

c732cc4

Added BigQuery Data type mappings

a459ab5

Add different tests for BigQuery

2ad0541

Added transactional sql executor for Big Query via Java API

dd14366

Add support for ParseTime , Fix tests, format code and add e2e Tests

12b309e

Rebase with Master and fix the issues

a50db2b

Dedicated Visitor for Derived Dataset to staging filtering for all sc…

35c1955

…hemes. Added tests for Big Query

Fix imports in the component module

c9ca920

Fix execute statements for DDL statements

5c9979d

Moved Connection inside relational sink for executor

18c2ef4

Fix Big Query Sink

d5ca20a

prasar-ashutosh force-pushed the big_query_support branch from a2ae98d to d5ca20a Compare June 8, 2023 03:52

prasar-ashutosh and others added 13 commits June 8, 2023 11:54

Bump version for big-query module pom

21f3dd8

Moved Connection inside relational sinks

508370f

Clean up interfaces to use Wrapper of Connection

6a23b0a

Format all Sink Classes

6320261

Initial Code for big query module

57ecb51

Added different Visitors for BigQuerySink

c90acfa

Added BigQuery Data type mappings

bcd4c60

Add different tests for BigQuery

31e6d5b

Added transactional sql executor for Big Query via Java API

d2c13f1

Add support for ParseTime , Fix tests, format code and add e2e Tests

28da86e

Rebase with Master and fix the issues

e68b762

Dedicated Visitor for Derived Dataset to staging filtering for all sc…

9345f66

…hemes. Added tests for Big Query

Fix imports in the component module

68637bd

prasar-ashutosh changed the title ~~DRAFT: Persistence Component - Big query support~~ Persistence Component - Big query support Jun 15, 2023

Add primary keys fetch and validation

873b6ea

davidsunglee approved these changes Jun 27, 2023

View reviewed changes

shubham43 added 6 commits June 28, 2023 09:52

Add schema evolution tests and bitemporal delta end-to-end test

bae9c95

Update alter table command after rebase

0a2f9b0

Revert unintentional changes from rebase

a4f312c

Added Bitemporal Delta end-to-end test

a3c43f3

Fix insert sql test

22aa57f

kumuwu reviewed Jun 30, 2023

View reviewed changes

shubham43 added 2 commits June 30, 2023 19:14

Reduced entries in implicit data type mapping, and renamed evolveFiel…

f831778

…d method params

Removed finished todos

462f444

prasar-ashutosh closed this Jun 30, 2023

prasar-ashutosh reopened this Jun 30, 2023

prasar-ashutosh and others added 4 commits June 30, 2023 22:16

Bump version of parent in Big Query pom

ce674f4

Add documentation for getting service account credential path

c7bb28e

Merge remote-tracking branch 'origin/big_query_support' into big_quer…

aa58fad

…y_support

Fixed tests post schema column rename

def4c96

prasar-ashutosh closed this Jun 30, 2023

prasar-ashutosh reopened this Jun 30, 2023

prasar-ashutosh and others added 5 commits June 30, 2023 23:05

Merge branch 'finos:master' into big_query_support

e20c620

Merge branch 'finos:master' into big_query_support

314a5af

Create logical plan to fetch primary keys in Big Query

4996f9d

Merge remote-tracking branch 'origin/big_query_support' into big_quer…

70eba5e

…y_support

Upgrade pom to latest

6372057

rafaelbey reviewed Jul 3, 2023

View reviewed changes

Handle code review comments

ae4b796

epsstan approved these changes Jul 4, 2023

View reviewed changes

epsstan merged commit 594db31 into finos:master Jul 4, 2023
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Persistence Component - Big query support #1904

Persistence Component - Big query support #1904

prasar-ashutosh commented Jun 8, 2023

linux-foundation-easycla bot commented Jun 8, 2023 •

edited

Loading

github-actions bot commented Jun 8, 2023 •

edited

Loading

TheJuanAndOnly99 commented Jun 26, 2023

kumuwu Jun 30, 2023

shubham43 Jun 30, 2023

rafaelbey Jul 3, 2023

prasar-ashutosh Jul 4, 2023

rafaelbey Jul 3, 2023

prasar-ashutosh Jul 4, 2023

rafaelbey Jul 3, 2023

prasar-ashutosh Jul 4, 2023

rafaelbey Jul 3, 2023

rafaelbey Jul 3, 2023

prasar-ashutosh Jul 4, 2023

rafaelbey Jul 3, 2023

prasar-ashutosh Jul 4, 2023 •

edited

Loading

rafaelbey Jul 3, 2023

prasar-ashutosh Jul 4, 2023

Persistence Component - Big query support #1904

Persistence Component - Big query support #1904

Conversation

prasar-ashutosh commented Jun 8, 2023

What type of PR is this?

What does this PR do / why is it needed ?

Which issue(s) this PR fixes:

Other notes for reviewers:

Does this PR introduce a user-facing change?

linux-foundation-easycla bot commented Jun 8, 2023 • edited Loading

github-actions bot commented Jun 8, 2023 • edited Loading

Test Results

TheJuanAndOnly99 commented Jun 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

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

prasar-ashutosh Jul 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linux-foundation-easycla bot commented Jun 8, 2023 •

edited

Loading

github-actions bot commented Jun 8, 2023 •

edited

Loading

prasar-ashutosh Jul 4, 2023 •

edited

Loading