Fix Iceberg's parquet reader returning nulls incorrectly for parquet files written by writers that don't use list and element as names. #3723

SinghAsDev · 2021-12-13T00:21:49Z

Parquet files written with Parquet writers that use names other than list and element for repeated group
and element of the list respectively are read incorrectly. Parquet files generated by Hive fall in this category as well.

This PR fixes Iceberg's parquet writer returning nulls incorrectly for such parquet files and updates migration doc to inform users of this issue.

SinghAsDev · 2021-12-13T00:28:43Z

@RussellSpitzer @rdblue what do you think about this, while I work on adding backwards compatibility support?

kbendick

Thanks for reporting this @SinghAsDev.

I noticed that the Parquet file you submitted is using what looks like a custom Parquet version.

$ parquet-tools meta spark/v3.2/spark/src/test/resources/twoLevelList.pq
creator:         parquet-mr version 1.10.1-pinterest-0.0.75 (build ${buildNumber})

file schema:     hive_schema
--------------------------------------------------------------------------------
key:             OPTIONAL BINARY L:STRING R:0 D:1
val:             OPTIONAL F:1
.bag:            REPEATED F:1
..array_element: OPTIONAL F:2
...a1:           OPTIONAL BINARY L:STRING R:1 D:4
...a2:           OPTIONAL BINARY L:STRING R:1 D:4

row group 1:     RC:1 TS:153 OFFSET:4
--------------------------------------------------------------------------------
key:              BINARY ZSTD DO:0 FPO:4 SZ:58/49/0.84 VC:1 ENC:BIT_PACKED,PLAIN,RLE ST:[min: k1, max: k1, num_nulls: 0]
val:
.bag:
..array_element:
...a1:            BINARY ZSTD DO:0 FPO:62 SZ:61/52/0.85 VC:1 ENC:PLAIN,RLE ST:[min: a, max: a, num_nulls: 0]
...a2:            BINARY ZSTD DO:0 FPO:123 SZ:61/52/0.85 VC:1 ENC:PLAIN,RLE ST:[min: b, max: b, num_nulls: 0]

Does this also happen with OSS Iceberg? I will try to check. It would be great if instead of using a checked in file, Spark was used to generate the data instead (although I know that doesn't capture every case).

kbendick · 2021-12-13T02:07:13Z

spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/data/TestParquetReader.java

+    /* Using a static file rather than generating test data in test, as parquet writers in Iceberg only supports
+     * three level lists. The twoLevelList.pq is a parquet file that contains following Parquet schema.
+     * message hive_schema {
+     *  optional binary key (STRING);
+     *  optional group val (LIST) {
+     *    repeated group bag {
+     *      optional group array_element {
+     *        optional binary a1 (STRING);
+     *        optional binary a2 (STRING);
+     *      }
+     *    }
+     *  }
+     * }
+     *
+     * It contains only one row. Below is the json dump of the file.
+     * {"key":"k1","val":{"bag":[{"array_element":{"a1":"a","a2":"b"}}]}}
+     */


I think your schema is off vs your JSON element:

It looks like val should be a list, but val in the JSON is a struct.

I might have that wrong though. I'm still working on a reproduction.

When I read in the element from the file, using spark, it doesn't have the same shape as the JSON that you showed.

scala> val df = spark.read.parquet("./spark/v3.2/spark/src/test/resources/twoLevelList.pq") df: org.apache.spark.sql.DataFrame = [key: string, val: array<struct<a1:string,a2:string>>] scala> df.printSchema root |-- key: string (nullable = true) |-- val: array (nullable = true) | |-- element: struct (containsNull = true) | | |-- a1: string (nullable = true) | | |-- a2: string (nullable = true) scala> df.toJSON.show(false) +----------------------------------------+ |value | +----------------------------------------+ |{"key":"k1","val":[{"a1":"a","a2":"b"}]}| +----------------------------------------+

Yea, it depends on how you are getting the json dump. If you use parquet-tools and use cat --json you would get the json I had pasted here.

Yeah that is correct. I did get the json you posted here.

SinghAsDev · 2021-12-13T02:28:25Z

@kbendick thanks for quick review.

Does this also happen with OSS Iceberg?

Yes.

It would be great if instead of using a checked in file, Spark was used to generate the data instead (although I know that doesn't capture every case).

Yea, the issue is that this issue only happens for parquet files generated from old non iceberg parquet writer. I have also mentioned this as a comment in the test file.

RussellSpitzer · 2021-12-13T02:51:17Z

I think it's worthwhile to add backwards compatibility. For reference we discussed this on slack and I noted the one case where we hit this internally #2167 but there we ended up just writing our files without the old structure in the source of the files. We hadn't hit it since then even with various other older hadoop distributions lying around.

That said if other folks are hitting this we should fix it.

It would be great to know the exact versions of hive/parquet that are generating this structure.

kbendick

Ok. I have a reproduction that I will push to a separate branch.

I think it's because your schema has the word bag in it. I think this is one of those cases, like what happened with parquet-map type when it changed between 1.11.0 and 1.11.1, where we're relying on the parquet structure name.

When I run the same test with a file I generated with this schema (same data), I do not get null.

message spark_schema {
  required binary key (STRING);
  required group val (LIST) {
    repeated group list {
      optional group element {
        optional binary a1 (STRING);
        optional binary a2 (STRING);
      }
    }
  }
}

My file:

$ parquet-tools head spark/v3.2/spark/src/test/resources/parquet_test.parquet
key = k1
val:
.list:
..element:
...a1 = a
...a2 = b

Your file

$ parquet-tools head spark/v3.2/spark/src/test/resources/twoLevelList.pq
key = k1
val:
.bag:
..array_element:
...a1 = a
...a2 = b

Again, my element as JSON vs the element in the provided file as JSON:

{"key":"k1","val":{"list":[{"element":{"a1":"a","a2":"b"}}]}}

The one you provided

{"key":"k1","val":{"bag":[{"array_element":{"a1":"a","a2":"b"}}]}}

The difference is that yours is using the non-standard name bag. We applied a fix before where we can read any name (and don't rely on the parquet names for the field), so we can look into that tomorrow. I'll also check the discussion on slack.

But I'm wondering, how did your parquet file get generated with bag instead of list for the array element name? It seems the parquet version is parquet-mr version 1.10.1-pinterest-0.0.75 (build ${buildNumber}) as reported by the file, but what version (roughly speaking) of MR are you using?

SinghAsDev · 2021-12-13T03:33:14Z

bag is added by hive's schema converter (defined here. We use this serde from hive's 1.2.1 version. However, even latest versions still use bag here.

Another thing to note here is that even if we use list as name for repeated group of a list, I think Iceberg returns just empty list. I am working on a fix for this, along with backward compatibility support. Would love to get your reviews on that as well.

kbendick · 2021-12-13T03:46:20Z

"bag" is added by hive's parquet serde, ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java. https://github.com/apache/hive/blob/release-1.2.1/ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java#L53. We use this serde from hive's 1.2.1 version. However, even latest versions still use "bag", https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java.

Another thing to note here is that even if we use list as name for repeated group of a list, I think Iceberg returns just empty list. I am working on a fix for this, along with backward compatibility support. Would love to get your reviews on that as well.

I’d be happy to take a look when you have a fix. We applied a fix for Iceberg 0.12.1 where we stopped relying on the name provided in the parquet-schema for map fields.

I think something like that would be the best route forward so that we aren’t imposing any restrictions on the parquet provided names as we’ve observed these change across systems and parquet versions.

SinghAsDev · 2021-12-13T10:09:13Z

@kbendick @RussellSpitzer I updated the PR with fix for this issue. However, the fix is only for 3-level lists and I will work on compatibility support in a separate diff.

RussellSpitzer · 2021-12-13T14:01:45Z

site/docs/spark-procedures.md


+**Note** Parquet files written with Parquet writers that use names other than `list` and `element` for repeated group
+and element of the list respectively are **read incorrectly** by Iceberg upto 0.12.1 Iceberg versions. Parquet files
+generated by Hive fall in this category.


Can we try to get a version bound here? Ideally the oss hive version where this starts and stops being an issue?

I checked 3.x and it's still called bag.

Yea, some details here

use !!! Note for note.

Also "Parquet files generated by Hive" sounds a bit vague. I think we can specify example cases are Parquet files written using (1) org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe in Hive, or (2) spark.sql.parquet.writeLegacyFormat=true in Spark.

SinghAsDev · 2021-12-13T16:28:06Z

Hive still writes with same names, so I would say it is a problem for any parquet file generated by writing to a hive table with ParquetHiveSerDe.

On Mon, Dec 13, 2021 at 6:02 AM Russell Spitzer ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In site/docs/spark-procedures.md <#3723 (comment)>: > @@ -334,6 +334,10 @@ CALL catalog_name.system.rewrite_manifests('db.sample', false) The `snapshot` and `migrate` procedures help test and migrate existing Hive or Spark tables to Iceberg. +**Note** Parquet files written with Parquet writers that use names other than `list` and `element` for repeated group +and element of the list respectively are **read incorrectly** by Iceberg upto 0.12.1 Iceberg versions. Parquet files +generated by Hive fall in this category. Can we try to get a version bound here? Ideally the oss hive version where this starts and stops being an issue? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3723 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABFQCZPI7FO53GWJLMU5M23UQX4FZANCNFSM5J43VMOA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

-- - Ashish

kbendick · 2021-12-13T19:43:34Z

Hive still writes with same names, so I would say it is a problem for any parquet file generated by writing to a hive table with ParquetHiveSerDe.
-- - Ashis

I suggest we consider going with the something similar to the map projection element rename fix that Ryan applied for maps from Parquet 1.11.0 to 1.11.1, where we don't depend on the name at all.

PR for context: #3309

It might not work because that was for projection, but it looks like you're updating similar areas of the code.

If we can get something like that where we don't assume names at all from parquet, just element structure and types, that would ideally allow for N levels of nesting etc. and further remove us from individual engines choice of element names for parquet (or changes within the parquet spec itself). cc @SinghAsDev

kbendick · 2021-12-13T19:48:37Z

parquet/src/main/java/org/apache/iceberg/parquet/ApplyNameMapping.java

+  private Type makeElement(Type element) {
+    // List's element in 3-level lists can be named differently across different parquet writers.
+    // For example, hive names it "array_element", whereas new parquet writers names it as "element".
+    if (element.getName().equals("element") || element.isPrimitive()) {
+      return element;
+    }


If it's possible to make this more generic and remove the dependency on the name entirely, that would be great.

Also, could you try to make a test that uses the logic in that file to design the schema instead of a checked in binary file? I know that might be difficult but if you wouldn't mind giving it a try, that would be preferred vs a binary file checked in.

As mentioned, here's a similar PR where you can find good test logic for making arbitrarily named Parquet message types: #3309

I think we can make it generic from the perspective that we don't have magic keywords like "element", however I am not sure if the projection change you are referring to would work here, but do let me know if you think otherwise. Here, we are making this change so that constructed parquet schema had correct ids, but still has the same schema as the one in parquet file. I will at least make this part generic, will update the diff shortly.

I tried a bit to remove the usage of "element", however there isn't a clean way to do so. The only way I could come up with is build a dummy list and then get the element out of it. I am not sure if that is any cleaner than the current approach. Below is the change I am referring to.

private Type makeElement(Type element) { // List's element in 3-level lists can be named differently across different parquet writers. // For example, hive names it "array_element", whereas new parquet writers names it as "element". if (element.isPrimitive()) { return element; } Types.BaseListBuilder.GroupElementBuilder<GroupType, Types.ListBuilder<GroupType>> dummyBuilder = Types.list(Type.Repetition.OPTIONAL) .groupElement(element.getRepetition()) .addFields(element.asGroupType().getFields().toArray(new Type[0])); if (element.getId() != null) { dummyBuilder.id(element.getId().intValue()); } return dummyBuilder.named("dummy").getType(0).asGroupType().getType(0); }

As such, I don't have a strong preference on it. @kbendick let me know if you like this one better and I will make the change.

@kbendick I also updated test to dynamically generate parquet file in required format. Mind giving it another pass.

Yeah I'll absolutely give this another pass. Sorry I was head down in something else today.

I'll also look for somebody to start the tests for you unless you'd like to wait. 😄

And yeah, understood the projection related changes, while similar in concept, don't necessarily apply here unfortunately.

Thanks @kbendick , getting the tests started will definitely be helpful.

The updated tests look great @SinghAsDev! Thank you!

kbendick

@SinghAsDev Two questions.

Are you using Hive on MapReduce or Hive on Tez?

And which Hive version did you say you were running? (I know that file still uses bag in later versions but just for my own knowledge so I can repro properly if need be).

SinghAsDev · 2021-12-14T04:32:00Z

@kbendick we were using Hive on MR with Hive 1.2.1 and so still have tons of files written by it. However, as we discussed this is not specific to old versions.

SinghAsDev · 2021-12-14T18:53:07Z

Hey @kbendick @RussellSpitzer would you be able to kick start the builds again? I had to fix a couple of checkStyle errors.

jackye1995 · 2021-12-16T18:19:37Z

"bag" is added by hive's parquet serde, ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java

So sounds like the issue will only come when users load table through Hive and import to Iceberg through add-files procedure. Technically users could have a way out by using migrate-table procedure to rewrite the entire table using the Iceberg writer instead.

But this seems to impact quite a lot of users for fast Hive to Iceberg migration, so I will add this in 0.13 milestone for now, please let me know if anyone objects.

RussellSpitzer · 2021-12-16T22:57:50Z

spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/data/TestParquetReader.java

+
+    // generate parquet file with required schema
+    List<String> testData = Collections.singletonList("{\"col1\": [{\"col2\": 1}]}");
+    spark.read().schema(sparkSchema).json(


Could we just run a SQL Insert statement here?

RussellSpitzer · 2021-12-16T23:03:51Z

spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/data/TestParquetReader.java

+  @Test
+  public void testHiveStyleThreeLevelList() throws IOException {
+    File location = new File(temp.getRoot(), "parquetReaderTest");
+    StructType sparkSchema =


So this is a different schema than would be created by

CREATE HIVEFORMAT TABLE x ( col_1 : List<struct<col2: int>>)

and

CREATE TABLE x ( col_1 : List<struct<col2: int>>) ?

I think the reason I had explicitly defined the schemas here is to demonstrate the issue a bit more clearly. However, I think now that we all are on same page in terms of the issue, I can make the tests a bit easy to follow. Will follow up with an update shortly.

RussellSpitzer · 2021-12-16T23:06:28Z

spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/data/TestParquetReader.java

+    Assert.assertEquals(expectedParquetSchema, schema.toString());
+
+    // read from Iceberg's parquet reader and ensure data is read correctly into Spark's internal row
+    Schema icebergSchema = SparkSchemaUtil.convert(sparkSchema);


I think we should do a test to make sure files imported are read correctly, instead of using the internal methods doing an import files into an iceberg table and showing that the rows are correct? Part of add_files or Migrate tests?

We should also have the low level tests in TestParquetSchemaUtil.java

Good idea, will update shortly.

SinghAsDev · 2021-12-17T02:23:44Z

@RussellSpitzer mind giving it another pass, thanks

RussellSpitzer · 2021-12-17T15:41:04Z

parquet/src/test/java/org/apache/iceberg/parquet/TestParquetSchemaUtil.java

+  @Test
+  public void testSchemaConversionForHiveStyleLists() {
+    String parquetSchemaString =
+        "message spark_schema {\n" +


This schema cannot be constructed using the Parquet API directly? I was hoping this would look similar to the tests above that create the schema using Parquet directly.

Unfortunately no, as the moment we call list, it will use 3-level list with list and element names

SinghAsDev · 2021-12-17T19:50:16Z

@RussellSpitzer @jackye1995 thanks for your reviews, the diff is updated to address your suggestions.

jackye1995 · 2021-12-17T20:29:22Z

site/docs/spark-procedures.md

+!!! Note
+    Parquet files written with Parquet writers that use names other than `list` and `element` for repeated group
+    and element of the list respectively are **read incorrectly as nulls** by Iceberg upto 0.12.1 Iceberg versions.
+    Most commonly such files are written by follow writers. 


nit: the following writers:

btw, have you tried mkdocs serve? Not sure if the indentation would be correctly displayed, please verify that.

Hey @jackye1995 , to ease the cherry-picks we decided to extract out format change to a separate PR. Will follow up on this right after we get this PR in.

sure, makes sense

Updated docs to take of nit comment and made sure it looks as expected with mkdocs serve as well.

for fields with 2-level lists. Also, update migrate tables documentation to let users know of the issue.

RussellSpitzer

Lgtm +1, Will merge when tests finish in a second

RussellSpitzer · 2021-12-17T22:01:15Z

Thanks so much to @SinghAsDev for the PR and @jackye1995 and @kbendick for the review

SinghAsDev · 2021-12-17T22:04:13Z

Thanks for reviews @RussellSpitzer @jackye1995 @kbendick !

…ns. (#3767) * Add spark's legacy mode write tests to spark3.0 and 3.1 versions backport of tests from #3723

* apache/iceberg#3723 * apache/iceberg#3732 * apache/iceberg#3749 * apache/iceberg#3766 * apache/iceberg#3787 * apache/iceberg#3796 * apache/iceberg#3809 * apache/iceberg#3820 * apache/iceberg#3878 * apache/iceberg#3890 * apache/iceberg#3892 * apache/iceberg#3944 * apache/iceberg#3976 * apache/iceberg#3993 * apache/iceberg#3996 * apache/iceberg#4008 * apache/iceberg#3758 and 3856 * apache/iceberg#3761 * apache/iceberg#2062 * apache/iceberg#3422 * remove restriction related to legacy parquet file list

github-actions bot added build docs spark labels Dec 13, 2021

kbendick reviewed Dec 13, 2021

View reviewed changes

github-actions bot added the parquet label Dec 13, 2021

SinghAsDev changed the title ~~Demonstrate issue where Iceberg's parquet reader silently returns nulls~~ Fix Iceberg's parquet writer returning nulls incorrectly for parquet files written by writers that don't use list and element as names. Dec 13, 2021

RussellSpitzer reviewed Dec 13, 2021

View reviewed changes

kbendick reviewed Dec 13, 2021

View reviewed changes

kbendick reviewed Dec 14, 2021

View reviewed changes

jackye1995 added this to the Iceberg 0.13.0 Release milestone Dec 16, 2021

SinghAsDev mentioned this pull request Dec 16, 2021

Add backwards compatibility read support for Parquet list #3759

Closed

RussellSpitzer reviewed Dec 16, 2021

View reviewed changes

RussellSpitzer reviewed Dec 17, 2021

View reviewed changes

jackye1995 reviewed Dec 17, 2021

View reviewed changes

SinghAsDev added 14 commits December 17, 2021 12:51

Demonstrate issue where Iceberg's parquet reader silently returns nulls

ad893cf

for fields with 2-level lists. Also, update migrate tables documentation to let users know of the issue.

Undo copy error

5953516

Drop dependency that is no longer needed

9ea100a

2

c192ad8

update doc

c236492

Remove unused import

ef0e6fe

Nit

c05b826

Update test to not use static binary file

e2653de

Fix checkstyle errors

87fc0bc

Update unit tests

c87b1b5

Move tests into TestCreateActions

a6272e9

Update doc

d551cee

Add tests to also test files written in no-legacy mdoe

bfece89

Rebase

9bf3acf

SinghAsDev force-pushed the pq_test branch from 2bbe7cb to 9bf3acf Compare December 17, 2021 20:56

RussellSpitzer approved these changes Dec 17, 2021

View reviewed changes

RussellSpitzer merged commit 9b0aba7 into apache:master Dec 17, 2021

SinghAsDev deleted the pq_test branch December 17, 2021 22:38

RussellSpitzer pushed a commit that referenced this pull request Dec 18, 2021

Spark: Add spark's legacy mode write tests to spark3.0 and 3.1 versio…

c5d3d1c

…ns. (#3767) * Add spark's legacy mode write tests to spark3.0 and 3.1 versions backport of tests from #3723

rdblue added a commit to rdblue/iceberg that referenced this pull request Dec 19, 2021

Parquet: Simplify changes from apache#3723.

dc3fe0e

rdblue added a commit to rdblue/iceberg that referenced this pull request Dec 19, 2021

Parquet: Simplify changes from apache#3723.

0b1ffe1

rdblue mentioned this pull request Dec 19, 2021

Parquet: Simplify changes from #3723 #3773

Merged

zhongyujiang mentioned this pull request Dec 20, 2021

make ApplyNameMapping keep field's name after name mapping. #3605

Closed

rdblue added a commit that referenced this pull request Dec 21, 2021

Parquet: Simplify changes from #3723 (#3773)

a0b57d5

jackye1995 pushed a commit to jackye1995/iceberg-docs that referenced this pull request Feb 8, 2022

https://github.com/apache/iceberg/pull/3723

77bf1b5

Fix Iceberg's parquet reader returning nulls incorrectly for parquet files written by writers that don't use list and element as names. #3723

Fix Iceberg's parquet reader returning nulls incorrectly for parquet files written by writers that don't use list and element as names. #3723

Uh oh!

Conversation

SinghAsDev commented Dec 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SinghAsDev commented Dec 13, 2021

Uh oh!

kbendick left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SinghAsDev commented Dec 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RussellSpitzer commented Dec 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kbendick left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SinghAsDev commented Dec 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kbendick commented Dec 13, 2021

Uh oh!

SinghAsDev commented Dec 13, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kbendick Dec 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SinghAsDev commented Dec 13, 2021 via email

Uh oh!

kbendick commented Dec 13, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kbendick left a comment

Choose a reason for hiding this comment

Uh oh!

SinghAsDev commented Dec 14, 2021

Uh oh!

SinghAsDev commented Dec 14, 2021

Uh oh!

jackye1995 commented Dec 16, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SinghAsDev commented Dec 13, 2021 •

edited

Loading

SinghAsDev commented Dec 13, 2021 •

edited

Loading

RussellSpitzer commented Dec 13, 2021 •

edited

Loading

kbendick left a comment •

edited

Loading

SinghAsDev commented Dec 13, 2021 •

edited

Loading

kbendick Dec 13, 2021 •

edited

Loading

RussellSpitzer Dec 16, 2021 •

edited

Loading