[SPARK-27494][SS] Null keys/values don't work in Kafka source v2 #24441

uncleGen · 2019-04-23T07:35:39Z

What changes were proposed in this pull request?

Right now Kafka source v2 doesn't support null keys or values.

When processing a null key, all of the following keys in the same partition will be null. This is a correctness bug.
When processing a null value, it will throw NPE.

How was this patch tested?

add new unit tests

SparkQA · 2019-04-23T08:02:27Z

Test build #104829 has finished for PR 24441 at commit 5cb9502.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

...-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/KafkaRecordToUnsafeRowConverter.scala

uncleGen · 2019-04-23T08:39:24Z

pending on adding new unit test

...-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/KafkaRecordToUnsafeRowConverter.scala

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaContinuousSourceSuite.scala

SparkQA · 2019-04-24T07:05:01Z

Test build #104852 has finished for PR 24441 at commit 74bfb87.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-04-24T07:05:01Z

Test build #104858 has finished for PR 24441 at commit aadad55.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

uncleGen · 2019-04-24T07:54:19Z

retest this please

SparkQA · 2019-04-24T11:02:39Z

Test build #104864 has finished for PR 24441 at commit aadad55.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

...-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/KafkaRecordToUnsafeRowConverter.scala

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaContinuousSourceSuite.scala

cloud-fan · 2019-04-24T15:26:12Z

good catch!

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaContinuousSourceSuite.scala

SparkQA · 2019-04-25T03:05:06Z

Test build #104885 has finished for PR 24441 at commit b6e9e31.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-04-25T03:24:37Z

Test build #104886 has finished for PR 24441 at commit 2f30711.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaMicroBatchSourceSuite.scala

SparkQA · 2019-04-25T04:18:50Z

Test build #104889 has finished for PR 24441 at commit 9dd38eb.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaMicroBatchSourceSuite.scala

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaContinuousSourceSuite.scala

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaMicroBatchSourceSuite.scala

SparkQA · 2019-04-25T05:09:23Z

Test build #104891 has finished for PR 24441 at commit 45330de.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-04-25T05:21:41Z

Test build #104893 has finished for PR 24441 at commit d91977d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2019-04-25T06:12:27Z

Although SPARK-27494 is reported on only v2, can we have a test coverage for Kafka source v1 too?

uncleGen · 2019-04-25T07:08:36Z

@dongjoon-hyun ~~make sense, what about finishing it in a follow-up pr?~~ v1 is already tested in KafkaMicroBatchV1SourceSuite

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaMicroBatchSourceSuite.scala

cloud-fan · 2019-04-25T07:42:58Z

@dongjoon-hyun v1 is already tested because the test is in KafkaMicroBatchSourceSuiteBase

gaborgsomogyi

Minor things found, basically looks good.

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaContinuousSourceSuite.scala

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaMicroBatchSourceSuite.scala

dongjoon-hyun · 2019-04-25T15:51:16Z

Got it. Thanks, @uncleGen and @cloud-fan !

SparkQA · 2019-04-26T02:46:13Z

Test build #104920 has finished for PR 24441 at commit d2ffea1.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-04-26T03:44:05Z

Test build #104921 has finished for PR 24441 at commit 174321a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

uncleGen · 2019-04-26T06:02:50Z

@cloud-fan Take a second glance please.

zsxwing

LGTM

## What changes were proposed in this pull request? Right now Kafka source v2 doesn't support null values. The issue is in org.apache.spark.sql.kafka010.KafkaRecordToUnsafeRowConverter.toUnsafeRow which doesn't handle null values. ## How was this patch tested? add new unit tests Closes #24441 from uncleGen/SPARK-27494. Authored-by: uncleGen <hustyugm@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit d2656aa) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

cloud-fan · 2019-04-26T06:28:51Z

thanks, merging to master/2.4!

dongjoon-hyun · 2019-04-26T16:20:58Z

Thank you all!

## What changes were proposed in this pull request? Right now Kafka source v2 doesn't support null values. The issue is in org.apache.spark.sql.kafka010.KafkaRecordToUnsafeRowConverter.toUnsafeRow which doesn't handle null values. ## How was this patch tested? add new unit tests Closes apache#24441 from uncleGen/SPARK-27494. Authored-by: uncleGen <hustyugm@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit d2656aa) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

uncleGen added 2 commits April 23, 2019 15:34

Null values don't work in Kafka source v2

a3adc70

minor update

5cb9502

uncleGen changed the title ~~Null values don't work in Kafka source v2~~ [SPARK-27494][SS] Null values don't work in Kafka source v2 Apr 23, 2019

dongjoon-hyun reviewed Apr 23, 2019

View reviewed changes

...-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/KafkaRecordToUnsafeRowConverter.scala Show resolved Hide resolved

add unit test

74bfb87

uncleGen commented Apr 24, 2019

View reviewed changes

...-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/KafkaRecordToUnsafeRowConverter.scala Outdated Show resolved Hide resolved

dongjoon-hyun reviewed Apr 24, 2019

View reviewed changes

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaContinuousSourceSuite.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Apr 24, 2019

View reviewed changes

...-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/KafkaRecordToUnsafeRowConverter.scala Show resolved Hide resolved

cloud-fan reviewed Apr 24, 2019

View reviewed changes

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaContinuousSourceSuite.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Apr 25, 2019

View reviewed changes

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaContinuousSourceSuite.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Apr 25, 2019

View reviewed changes

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaMicroBatchSourceSuite.scala Outdated Show resolved Hide resolved

dongjoon-hyun reviewed Apr 25, 2019

View reviewed changes

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaMicroBatchSourceSuite.scala Outdated Show resolved Hide resolved

fix comments

45330de

uncleGen force-pushed the SPARK-27494 branch from d8bfb49 to 45330de Compare April 25, 2019 04:37

viirya reviewed Apr 25, 2019

View reviewed changes

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaContinuousSourceSuite.scala Outdated Show resolved Hide resolved

viirya reviewed Apr 25, 2019

View reviewed changes

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaMicroBatchSourceSuite.scala Outdated Show resolved Hide resolved

fix comments

d91977d

cloud-fan reviewed Apr 25, 2019

View reviewed changes

...kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaMicroBatchSourceSuite.scala Outdated Show resolved Hide resolved

uncleGen closed this Apr 25, 2019

uncleGen reopened this Apr 25, 2019

gaborgsomogyi reviewed Apr 25, 2019

View reviewed changes

fix comments

174321a

uncleGen force-pushed the SPARK-27494 branch from d2ffea1 to 174321a Compare April 26, 2019 03:13

zsxwing approved these changes Apr 26, 2019

View reviewed changes

cloud-fan closed this in d2656aa Apr 26, 2019

zsxwing changed the title ~~[SPARK-27494][SS] Null values don't work in Kafka source v2~~ [SPARK-27494][SS] Null keys/values don't work in Kafka source v2 Apr 26, 2019

[SPARK-27494][SS] Null keys/values don't work in Kafka source v2 #24441

[SPARK-27494][SS] Null keys/values don't work in Kafka source v2 #24441

Uh oh!

Conversation

uncleGen commented Apr 23, 2019 • edited by zsxwing Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented Apr 23, 2019

Uh oh!

Uh oh!

uncleGen commented Apr 23, 2019

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Apr 24, 2019

Uh oh!

SparkQA commented Apr 24, 2019

Uh oh!

uncleGen commented Apr 24, 2019

Uh oh!

SparkQA commented Apr 24, 2019

Uh oh!

Uh oh!

Uh oh!

cloud-fan commented Apr 24, 2019

Uh oh!

Uh oh!

SparkQA commented Apr 25, 2019

Uh oh!

SparkQA commented Apr 25, 2019

Uh oh!

Uh oh!

SparkQA commented Apr 25, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Apr 25, 2019

Uh oh!

SparkQA commented Apr 25, 2019

Uh oh!

dongjoon-hyun commented Apr 25, 2019

Uh oh!

uncleGen commented Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

cloud-fan commented Apr 25, 2019

Uh oh!

gaborgsomogyi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dongjoon-hyun commented Apr 25, 2019

Uh oh!

SparkQA commented Apr 26, 2019

Uh oh!

SparkQA commented Apr 26, 2019

Uh oh!

uncleGen commented Apr 26, 2019

Uh oh!

zsxwing left a comment

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Apr 26, 2019

Uh oh!

dongjoon-hyun commented Apr 26, 2019

Uh oh!

Uh oh!

uncleGen commented Apr 23, 2019 •

edited by zsxwing

Loading

uncleGen commented Apr 25, 2019 •

edited

Loading