[KYUUBI #5317] [Bug] Hive Connector throws NotSerializableException on reading Hive Avro partitioned table #5319

zhaomin1423 · 2023-09-20T16:54:08Z

Why are the changes needed?

close #5317 (comment)

How was this patch tested?

Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request

Was this patch authored or co-authored using generative AI tooling?

No

…tion on reading Hive Avro partitioned table

codecov-commenter · 2023-09-20T18:27:59Z

Codecov Report

Merging #5319 (7944734) into master (cd325b4) will not change coverage.
Report is 2 commits behind head on master.
The diff coverage is 0.00%.

❗ Current head 7944734 differs from pull request most recent head d38832f. Consider uploading reports for the commit d38832f to get more accurate results

@@          Coverage Diff           @@
##           master   #5319   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         590     590           
  Lines       33429   33432    +3     
  Branches     4423    4422    -1     
======================================
- Misses      33429   33432    +3

Files Changed	Coverage Δ
...uubi/spark/connector/hive/read/HiveFileIndex.scala	`0.00% <0.00%> (ø)`
...nnector/hive/read/HivePartitionReaderFactory.scala	`0.00% <0.00%> (ø)`
...he/kyuubi/spark/connector/hive/read/HiveScan.scala	`0.00% <0.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

pan3793 · 2023-09-21T02:10:09Z

...rk-connector-hive/src/test/scala/org/apache/kyuubi/spark/connector/hive/HiveQuerySuite.scala

         | CREATE TABLE IF NOT EXISTS
         | $table (id String, year String, month string)
-         | USING PARQUET
+         | USING $format


I'm not sure how USING would be handled in this connector, I verified with STORED AS instead

Both work and also add unit tests

As far as i known, This point depend on the implementation of the catalog. v2HiveCatalog creates tables and queries them, in fact, STORED AS & USING are the same thing.

There is a small difference, the format of text does not work when STORED AS is used, only textfile, but USING work well.

this is caused by the parsing of spark, In spark, text file format is text, hive stored as is textfile

pan3793 · 2023-09-21T02:10:46Z

...rk-connector-hive/src/test/scala/org/apache/kyuubi/spark/connector/hive/HiveQuerySuite.scala

    }
  }
+
+  test("read partitioned avro table") {


let's also add test for un-partitioned serde table

Done, and add other formats.

yikf · 2023-09-21T03:01:35Z

@zhaomin1423 Thanks, LGTM

ulysses-you · 2023-09-21T03:14:59Z

lgtm

pan3793 · 2023-09-21T03:20:52Z

...rk-connector-hive/src/test/scala/org/apache/kyuubi/spark/connector/hive/HiveQuerySuite.scala

+    val clause = if (hiveTable) {
+      "STORED AS"
+    } else {
+      "USING"
+    }


let's inline it

pan3793 · 2023-09-21T03:21:34Z

.../src/main/scala/org/apache/kyuubi/spark/connector/hive/read/HivePartitionReaderFactory.scala

+        val hivePartition = if (bindHivePart.isDefined) {
+          HiveClientImpl.toHivePartition(bindHivePart.get, hiveTable)
+        } else {
+          null
+        }


Suggested change

val hivePartition = if (bindHivePart.isDefined) {

HiveClientImpl.toHivePartition(bindHivePart.get, hiveTable)

} else {

null

}

bindHivePart.map(HiveClientImpl.toHivePartition(_, hiveTable)).orNull

pan3793

LGTM, except to small nits

zhaomin1423 · 2023-09-21T03:40:15Z

LGTM, except to small nits

Update, thanks.

pan3793 · 2023-09-21T08:10:43Z

.../src/main/scala/org/apache/kyuubi/spark/connector/hive/read/HivePartitionReaderFactory.scala

              broadcastHiveConf,
              nonPartitionReadDataKeys,
-              bindHivePart,
+              hivePartition,


Emm... seems we can refactor it to a Option[HivePartition] to avoid null propagation

pan3793 · 2023-09-21T08:20:26Z

.../src/main/scala/org/apache/kyuubi/spark/connector/hive/read/HivePartitionReaderFactory.scala

-      bindPartition: HivePartition): Iterator[Writable] = {
+      bindPartition: Option[HivePartition]): Iterator[Writable] = {
    // Obtain binding HivePartition from input partitioned file
    val partDesc =


use Option too

pan3793 · 2023-09-21T09:05:13Z

The last two commits only change the code style, merging to master/1.8

…n reading Hive Avro partitioned table ### _Why are the changes needed?_ close #5317 (comment) ### _How was this patch tested?_ - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No Closes #5319 from zhaomin1423/fixhive-connector. Closes #5317 02e5321 [Cheng Pan] nit cadabf4 [Cheng Pan] nit d38832f [zhaomin] improve ee5b62d [zhaomin] improve 7944734 [zhaomin] improve e3eca91 [zhaomin] add tests d9302e2 [zhaomin] [KYUUBI #5317] [Bug] Hive Connector throws NotSerializableException on reading Hive Avro partitioned table 0bc8ec1 [zhaomin] [KYUUBI #5317] [Bug] Hive Connector throws NotSerializableException on reading Hive Avro partitioned table Lead-authored-by: zhaomin <zhaomin1423@163.com> Co-authored-by: Cheng Pan <chengpan@apache.org> Signed-off-by: Cheng Pan <chengpan@apache.org> (cherry picked from commit 167e6c1) Signed-off-by: Cheng Pan <chengpan@apache.org>

…tion on reading Hive Avro partitioned table ### _Why are the changes needed?_ close apache#5317 (comment) ### _How was this patch tested?_ - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No Closes apache#5319 from zhaomin1423/fixhive-connector. Closes apache#5317 02e5321 [Cheng Pan] nit cadabf4 [Cheng Pan] nit d38832f [zhaomin] improve ee5b62d [zhaomin] improve 7944734 [zhaomin] improve e3eca91 [zhaomin] add tests d9302e2 [zhaomin] [KYUUBI apache#5317] [Bug] Hive Connector throws NotSerializableException on reading Hive Avro partitioned table 0bc8ec1 [zhaomin] [KYUUBI apache#5317] [Bug] Hive Connector throws NotSerializableException on reading Hive Avro partitioned table Lead-authored-by: zhaomin <zhaomin1423@163.com> Co-authored-by: Cheng Pan <chengpan@apache.org> Signed-off-by: Cheng Pan <chengpan@apache.org> (cherry picked from commit 167e6c1) Signed-off-by: Cheng Pan <chengpan@apache.org>

[KYUUBI apache#5317] [Bug] Hive Connector throws NotSerializableExcep…

0bc8ec1

…tion on reading Hive Avro partitioned table

github-actions bot added module:spark module:extensions labels Sep 20, 2023

zhaomin1423 changed the title ~~[KYUUBI #5317] [Bug] Hive Connector throws NotSerializableException o…~~ [KYUUBI #5317] [Bug] Hive Connector throws NotSerializableException on reading Hive Avro partitioned table Sep 20, 2023

zhaomin1423 requested review from pan3793 and yikf September 20, 2023 16:54

[KYUUBI apache#5317] [Bug] Hive Connector throws NotSerializableExcep…

d9302e2

…tion on reading Hive Avro partitioned table

pan3793 reviewed Sep 21, 2023

View reviewed changes

pan3793 requested a review from ulysses-you September 21, 2023 02:11

add tests

e3eca91

pan3793 reviewed Sep 21, 2023

View reviewed changes

pan3793 approved these changes Sep 21, 2023

View reviewed changes

improve

7944734

zhaomin1423 force-pushed the fixhive-connector branch from 21a857d to 7944734 Compare September 21, 2023 03:39

pan3793 reviewed Sep 21, 2023

View reviewed changes

improve

ee5b62d

zhaomin1423 self-assigned this Sep 21, 2023

pan3793 reviewed Sep 21, 2023

View reviewed changes

pan3793 added this to the v1.8.0 milestone Sep 21, 2023

cxzl25 approved these changes Sep 21, 2023

View reviewed changes

zhaomin1423 and others added 3 commits September 21, 2023 16:33

improve

d38832f

nit

cadabf4

nit

02e5321

pan3793 closed this in 167e6c1 Sep 21, 2023

[KYUUBI #5317] [Bug] Hive Connector throws NotSerializableException on reading Hive Avro partitioned table #5319

[KYUUBI #5317] [Bug] Hive Connector throws NotSerializableException on reading Hive Avro partitioned table #5319

Uh oh!

Conversation

zhaomin1423 commented Sep 20, 2023 • edited by pan3793 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are the changes needed?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

codecov-commenter commented Sep 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pan3793 Sep 21, 2023 • 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!

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!

yikf commented Sep 21, 2023

Uh oh!

ulysses-you commented Sep 21, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pan3793 left a comment

Choose a reason for hiding this comment

Uh oh!

zhaomin1423 commented Sep 21, 2023

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!

pan3793 commented Sep 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

zhaomin1423 commented Sep 20, 2023 •

edited by pan3793

Loading

codecov-commenter commented Sep 20, 2023 •

edited

Loading

pan3793 Sep 21, 2023 •

edited

Loading