Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-27735][SS] Parsing interval string should be case-insensitive in SS #24619

Closed
wants to merge 6 commits into from

Conversation

Projects
None yet
4 participants
@zsxwing
Copy link
Member

commented May 15, 2019

What changes were proposed in this pull request?

Some APIs in Structured Streaming requires the user to specify an interval. Right now these APIs don't accept upper-case strings.

This PR adds a new method fromCaseInsensitiveString to CalendarInterval to support paring upper-case strings, and fixes all APIs that need to parse an interval string.

How was this patch tested?

The new unit test.

zsxwing added some commits May 15, 2019

fix
fix
fix

@zsxwing zsxwing changed the title [SPARK-27735][SS]Parsing interval string should be case-sensitive in SS [SPARK-27735][SS]Parsing interval string should be case-insensitive in SS May 15, 2019

@zsxwing

This comment has been minimized.

Copy link
Member Author

commented May 15, 2019

@SparkQA

This comment has been minimized.

Copy link

commented May 15, 2019

Test build #105432 has finished for PR 24619 at commit 02f6f33.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.
@SparkQA

This comment has been minimized.

Copy link

commented May 15, 2019

Test build #105433 has finished for PR 24619 at commit 2141445.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.
fix
@SparkQA

This comment has been minimized.

Copy link

commented May 16, 2019

Test build #105436 has finished for PR 24619 at commit 50507c5.

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

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-27735][SS]Parsing interval string should be case-insensitive in SS [SPARK-27735][SS] Parsing interval string should be case-insensitive in SS May 16, 2019

@SparkQA

This comment has been minimized.

Copy link

commented May 16, 2019

Test build #105464 has finished for PR 24619 at commit 6e4b733.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
@dongjoon-hyun
Copy link
Member

left a comment

+1, LGTM.
Merged to master/branch-2.4/branch-2.3.

dongjoon-hyun added a commit that referenced this pull request May 16, 2019

[SPARK-27735][SS] Parsing interval string should be case-insensitive …
…in SS

Some APIs in Structured Streaming requires the user to specify an interval. Right now these APIs don't accept upper-case strings.

This PR adds a new method `fromCaseInsensitiveString` to `CalendarInterval` to support paring upper-case strings, and fixes all APIs that need to parse an interval string.

The new unit test.

Closes #24619 from zsxwing/SPARK-27735.

Authored-by: Shixiong Zhu <zsxwing@gmail.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
(cherry picked from commit 6a317c8)
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

dongjoon-hyun added a commit that referenced this pull request May 16, 2019

[SPARK-27735][SS] Parsing interval string should be case-insensitive …
…in SS

Some APIs in Structured Streaming requires the user to specify an interval. Right now these APIs don't accept upper-case strings.

This PR adds a new method `fromCaseInsensitiveString` to `CalendarInterval` to support paring upper-case strings, and fixes all APIs that need to parse an interval string.

The new unit test.

Closes #24619 from zsxwing/SPARK-27735.

Authored-by: Shixiong Zhu <zsxwing@gmail.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
(cherry picked from commit 6a317c8)
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

@zsxwing zsxwing deleted the zsxwing:SPARK-27735 branch May 17, 2019

mukulmurthy added a commit to delta-io/delta that referenced this pull request May 23, 2019

[SC-17875]Parsing interval Delta config should be case-insensitive
## What changes were proposed in this pull request?

Some Delta configs accept an interval string. However, they only accept lower case. This is inconvenient.

This PR forks `CalendarInterval.fromCaseInsensitiveString` from apache/spark#24619 and uses it to parse the interval string to support upper case. It also improves the error message when the input string is an invalid interval (Right now it just throws NPE).

## How was this patch tested?

New unit test.

Author: Shixiong Zhu <zsxwing@gmail.com>

#5213 is resolved by zsxwing/SC-17875.

GitOrigin-RevId: d4d9cc0c2684096812be5697d1c093f4b000ce51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.