feat: Encode resources with data #88

erezrokah · 2023-08-22T16:41:26Z

Part of #5

~~Still doesn't work but at least Arrow doesn't throw an exception~~

Added column metadata too, and had to change some of the scalars/types to use whatever Java type the Arrow vector expects

erezrokah · 2023-08-22T16:41:48Z

lib/build.gradle

@@ -64,7 +64,7 @@ dependencies {
 test {
    useJUnitPlatform()
    testLogging {
-        events "passed", "skipped", "failed"


I removed this because it makes it a pain to see the failing tests

erezrokah · 2023-08-22T16:42:05Z

lib/build.gradle

@@ -100,6 +100,7 @@ task runMemDBServe(type: JavaExec) {
    classpath = sourceSets.main.runtimeClasspath
    main = javaMainClass
    args = ["serve"]
+    jvmArgs = ["--add-opens=java.base/java.nio=ALL-UNNAMED"]


Needed this for the arrow serialization

erezrokah · 2023-08-22T16:43:35Z

lib/src/main/java/io/cloudquery/helper/ArrowHelper.java

-          writer.start();
-          writer.end();
-          return ByteString.copyFrom(out.toByteArray());
+      try (VectorSchemaRoot schemaRoot = VectorSchemaRoot.create(schema, bufferAllocator)) {


Apparently we need to close VectorSchemaRoot (which closes all the underlying vectors) to avoid memory leaks. It's only an issue when we set data on the vectors so we I didn't see it on the table encode before.

erezrokah · 2023-08-22T16:44:31Z

lib/src/main/java/io/cloudquery/helper/ArrowHelper.java

+      Field field =
+          new Field(
+              column.getName(),
+              new FieldType(!column.isNotNull(), column.getType(), null, metadata),


First argument of FieldType says if the field is nullable

erezrokah · 2023-08-23T11:34:19Z

Going to merge to unblock. I can follow up if something is missing

mnorbury

Great progress 🚀 - left some comments.

mnorbury · 2023-08-23T11:31:26Z

lib/src/main/java/io/cloudquery/helper/ArrowHelper.java

+    if (vector instanceof BigIntVector) {
+      ((BigIntVector) vector).set(0, (long) data);
+      return;
+    }


You can follow this pattern which we do elsewhere to remove the cast on the line below:

Suggested change

if (vector instanceof BigIntVector) {

((BigIntVector) vector).set(0, (long) data);

return;

}

if (vector instanceof BigIntVector bigIntVector) {

bigIntVector.set(0, (long) data);

return;

}

mnorbury · 2023-08-23T11:34:40Z

lib/src/main/java/io/cloudquery/helper/ArrowHelper.java

@@ -57,7 +167,15 @@ public static Schema toArrowSchema(Table table) {
    Field[] fields = new Field[columns.size()];
    for (int i = 0; i < columns.size(); i++) {
      Column column = columns.get(i);
-      Field field = Field.nullable(column.getName(), column.getType());
+      Map<String, String> metadata = new HashMap<>();
+      metadata.put(CQ_EXTENSION_UNIQUE, column.isUnique() ? "true" : "false");


Can this just be Boolean.toString(column.isUnique())?

mnorbury · 2023-08-23T11:36:17Z

lib/src/main/java/io/cloudquery/helper/ArrowHelper.java

    return new Schema(asList(fields), metadata);
  }

  public static Table fromArrowSchema(Schema schema) {
    List<Column> columns = new ArrayList<>();
    for (Field field : schema.getFields()) {
-      columns.add(Column.builder().name(field.getName()).type(field.getType()).build());
+      boolean isUnique = field.getMetadata().get(CQ_EXTENSION_UNIQUE) == "true";


Suggested change

boolean isUnique = field.getMetadata().get(CQ_EXTENSION_UNIQUE) == "true";

boolean isUnique = field.getMetadata().get(CQ_EXTENSION_UNIQUE).equals("true");

Or use Objects.equals(field.getMetadata().get(CQ_EXTENSION_UNIQUE), "true") if it can be null.

mnorbury · 2023-08-23T11:38:33Z

lib/src/main/java/io/cloudquery/scheduler/Scheduler.java

@@ -38,6 +37,10 @@ public void sync() {
    for (Table table : tables) {
      try {
        logger.info("resolving table: {}", table.getName());
+        if (table.getResolver() == null) {


We could change the getResolver to return Optional<Resolver> - to force clients to handle a potential null sceenario.

🤖 I have created a release *beep* *boop* --- ## 0.0.1 (2023-08-24) ### Features * `io.cloudquery.scalar.Binary` implementation ([#20](#20)) ([b9b73d1](b9b73d1)) * `io.cloudquery.scalar.Bool` ([#27](#27)) ([2a91c92](2a91c92)), closes [#26](#26) * adding basic support for tables ([#19](#19)) ([22b2350](22b2350)) * adding JSON scalar ([#82](#82)) ([fc92542](fc92542)), closes [#63](#63) * adding Table filterDFS functionaility ([#21](#21)) ([02d8515](02d8515)) * Date scalars ([#36](#36)) ([adc6ba2](adc6ba2)), closes [#34](#34) * Duration scalar ([#42](#42)) ([7529438](7529438)), closes [#39](#39) * Encode resources with data ([#88](#88)) ([2c7060f](2c7060f)) * Generics in scalars ([#56](#56)) ([bc7d6e3](bc7d6e3)) * Implement `getTables` ([#71](#71)) ([085c51f](085c51f)) * Implement concurrency and relations resolving ([#91](#91)) ([0a470b7](0a470b7)) * Init logger, add initial MemDB plugin ([#70](#70)) ([20ebb42](20ebb42)) * int/uint/float/string scalars ([#59](#59)) ([39ec6e6](39ec6e6)), closes [#53](#53) [#54](#54) [#58](#58) [#60](#60) * Resolve CQId, add CQIds to MemDB plugin ([#95](#95)) ([9d7f1bd](9d7f1bd)) * Scalar Timestamp ([#46](#46)) ([4220e92](4220e92)), closes [#44](#44) * **sync:** Initial insert message support ([#81](#81)) ([bd729bb](bd729bb)) * **sync:** Send migrate messages ([#79](#79)) ([dd2c1a5](dd2c1a5)) ### Bug Fixes * Add `jackson-annotations` to `build.gradle` ([#83](#83)) ([ead7dd9](ead7dd9)) * **deps:** Update dependency com.google.guava:guava to v32 ([#15](#15)) ([ce8028b](ce8028b)) * **deps:** Update dependency io.grpc:grpc-protobuf to v1.57.1 ([#10](#10)) ([bcfa29c](bcfa29c)) * **deps:** Update dependency io.grpc:grpc-services to v1.57.1 ([#11](#11)) ([71c2ea1](71c2ea1)) * **deps:** Update dependency io.grpc:grpc-stub to v1.57.1 ([#12](#12)) ([c65e5d6](c65e5d6)) * **deps:** Update dependency io.grpc:grpc-testing to v1.57.1 ([#13](#13)) ([a7b1fa6](a7b1fa6)) * **deps:** Update plugin org.gradle.toolchains.foojay-resolver-convention to v0.6.0 ([#14](#14)) ([443990c](443990c)) * Flatten tables in getTables gRPC call ([#80](#80)) ([8c9872a](8c9872a)) * Pass options to tables method ([#78](#78)) ([4b77a2f](4b77a2f)) ### Miscellaneous Chores * Release 0.0.1 ([e169dbc](e169dbc)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

erezrokah commented Aug 22, 2023

View reviewed changes

erezrokah added 3 commits August 23, 2023 12:42

feat: Encode resources with data

70f3bf8

fix: Put constraintName on the table, not column

f317d33

fix: Set value and row count

ac2e359

erezrokah force-pushed the feat/resolvers branch from 3536732 to ac2e359 Compare August 23, 2023 10:52

erezrokah marked this pull request as ready for review August 23, 2023 10:53

erezrokah requested a review from mnorbury August 23, 2023 10:53

erezrokah added the automerge Add to automerge PRs once requirements are met label Aug 23, 2023

erezrokah merged commit 2c7060f into main Aug 23, 2023

erezrokah deleted the feat/resolvers branch August 23, 2023 11:36

cq-bot mentioned this pull request Aug 23, 2023

chore(main): Release v0.0.1 #16

Merged

mnorbury reviewed Aug 23, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Encode resources with data #88

feat: Encode resources with data #88

erezrokah commented Aug 22, 2023 •

edited

Loading

erezrokah Aug 22, 2023

erezrokah Aug 22, 2023

erezrokah Aug 22, 2023

erezrokah Aug 22, 2023

erezrokah commented Aug 23, 2023

mnorbury left a comment

mnorbury Aug 23, 2023

mnorbury Aug 23, 2023

mnorbury Aug 23, 2023

mnorbury Aug 23, 2023

	boolean isUnique = field.getMetadata().get(CQ_EXTENSION_UNIQUE) == "true";
	boolean isUnique = field.getMetadata().get(CQ_EXTENSION_UNIQUE).equals("true");

feat: Encode resources with data #88

feat: Encode resources with data #88

Conversation

erezrokah commented Aug 22, 2023 • edited Loading

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

erezrokah commented Aug 23, 2023

mnorbury 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

erezrokah commented Aug 22, 2023 •

edited

Loading