Skip to content

Conversation

@gaborgsomogyi
Copy link
Contributor

@gaborgsomogyi gaborgsomogyi commented Aug 12, 2019

What changes were proposed in this pull request?

SPARK-28163 fixed a bug and during the analysis we've concluded it would be more robust to use CaseInsensitiveMap inside Kafka source. This case less lower/upper case problem would rise in the future.

Please note this PR doesn't intend to solve any kind of actual problem but finish the concept added in SPARK-28163 (in a fix PR I didn't want to add too invasive changes). In this PR I've changed Map[String, String] to CaseInsensitiveMap[String] to enforce the usage. These are the main use-cases:

  • contains => CaseInsensitiveMap solves it
  • get... => CaseInsensitiveMap solves it
  • filter => keys must be converted to lowercase because there is no guarantee that the incoming map has such key set
  • find => keys must be converted to lowercase because there is no guarantee that the incoming map has such key set
  • passing parameters to Kafka consumer/producer => keys must be converted to lowercase because there is no guarantee that the incoming map has such key set

How was this patch tested?

Existing unit tests.

@SparkQA
Copy link

SparkQA commented Aug 12, 2019

Test build #108969 has finished for PR 25418 at commit 2925640.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gaborgsomogyi gaborgsomogyi changed the title [SPARK-28695][SS] Use CaseInsensitiveMap in KafkaSourceProvider to make source param handling more robust [WIP][SPARK-28695][SS] Use CaseInsensitiveMap in KafkaSourceProvider to make source param handling more robust Aug 12, 2019
@gaborgsomogyi gaborgsomogyi changed the title [WIP][SPARK-28695][SS] Use CaseInsensitiveMap in KafkaSourceProvider to make source param handling more robust [SPARK-28695][SS] Use CaseInsensitiveMap in KafkaSourceProvider to make source param handling more robust Aug 12, 2019
@SparkQA
Copy link

SparkQA commented Aug 12, 2019

Test build #108971 has finished for PR 25418 at commit d5ba9f1.

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

@gaborgsomogyi
Copy link
Contributor Author

cc @HeartSaVioR @HyukjinKwon @cloud-fan this is a follow-up on #24967

@SparkQA
Copy link

SparkQA commented Aug 13, 2019

Test build #109039 has finished for PR 25418 at commit c0b945c.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in a493031 Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants