Skip to content

Conversation

@sarutak
Copy link
Member

@sarutak sarutak commented Oct 23, 2014

We cannot use EOL character like \n or \r in the operand of LIKE predicate.
So following condition is never true.

-- someStr is 'hoge\nfuga'
where someStr LIKE 'hoge_fuga'

@SparkQA
Copy link

SparkQA commented Oct 23, 2014

QA tests have started for PR 2908 at commit 38f6651.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Oct 23, 2014

QA tests have finished for PR 2908 at commit 38f6651.

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

@sarutak sarutak changed the title [SPARK-4061] We cannot use EOL character in the operand of LIKE predicate. [SPARK-4061][SQL] We cannot use EOL character in the operand of LIKE predicate. Oct 23, 2014
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel a little complicated about this... This function is not on critical path, so I'd like to refactor it in a more functional and arguably more readable (but less efficient) way, for example:

  override def escape(v: String) = "(?s)" + (' ' +: v.init).zip(v).flatMap {
    case (prefix, '_') => if (prefix == '\\') "_" else "."
    case (prefix, '%') => if (prefix == '\\') "%" else ".*"
    case (_, ch) => Character.toString(ch)
  }.mkString

@liancheng
Copy link
Contributor

Good catch! Would you mind to add a unit test for this?

@sarutak
Copy link
Member Author

sarutak commented Oct 24, 2014

Thanks @liancheng ! I've refactored and added test cases.

@SparkQA
Copy link

SparkQA commented Oct 24, 2014

Test build #22121 has finished for PR 2908 at commit d98c99e.

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

@SparkQA
Copy link

SparkQA commented Oct 24, 2014

Test build #22130 has finished for PR 2908 at commit 8a33f91.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this branch?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see, we need to remove leading \\.

@liancheng
Copy link
Contributor

LGTM, thanks for fixing this!

@SparkQA
Copy link

SparkQA commented Oct 25, 2014

Test build #22205 has finished for PR 2908 at commit 1051dd0.

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

@sarutak
Copy link
Member Author

sarutak commented Oct 25, 2014

retest this please.

@SparkQA
Copy link

SparkQA commented Oct 25, 2014

Test build #22214 has finished for PR 2908 at commit 1051dd0.

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

@sarutak
Copy link
Member Author

sarutak commented Oct 26, 2014

retest this please.

@SparkQA
Copy link

SparkQA commented Oct 26, 2014

Test build #22243 has finished for PR 2908 at commit 1051dd0.

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

@SparkQA
Copy link

SparkQA commented Oct 26, 2014

Test build #22245 has finished for PR 2908 at commit 6bc3318.

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

@JoshRosen
Copy link
Contributor

It looks like the build configuration for the thriftserver tests may be broken, which is why they're failing:

[error] Expected ID character
[error] Not a valid command: hive-thriftserver
[error] Expected project ID
[error] Expected configuration
[error] Expected ':' (if selecting a configuration)
[error] Expected key
[error] Not a valid key: hive-thriftserver
[error] hive-thriftserver/test
[error]                  ^
[error] Got a return code of 1 on line 193 of the run-tests script.
Archiving unit tests logs...

@sarutak
Copy link
Member Author

sarutak commented Oct 26, 2014

Thanks, I'll try to create another branch from master and then push.

@sarutak sarutak force-pushed the spark-sql-like-match-modification branch from 6bc3318 to f99a2f4 Compare October 26, 2014 20:43
@SparkQA
Copy link

SparkQA commented Oct 26, 2014

Test build #22254 has finished for PR 2908 at commit f99a2f4.

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

@sarutak
Copy link
Member Author

sarutak commented Oct 26, 2014

Ah, I see. It's related to #2950.

@SparkQA
Copy link

SparkQA commented Oct 26, 2014

Test build #453 has finished for PR 2908 at commit f99a2f4.

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

@SparkQA
Copy link

SparkQA commented Oct 26, 2014

Test build #22260 has finished for PR 2908 at commit d15798b.

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

@SparkQA
Copy link

SparkQA commented Oct 26, 2014

Test build #464 has finished for PR 2908 at commit d15798b.

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

@marmbrus
Copy link
Contributor

Thanks for working on this! Merged to master.

@asfgit asfgit closed this in 3a9d66c Oct 26, 2014
@sarutak sarutak deleted the spark-sql-like-match-modification branch April 11, 2015 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants