Skip to content

Conversation

@jonahgao
Copy link
Member

@jonahgao jonahgao commented Apr 26, 2024

Which issue does this PR close?

Closes #10240.

Rationale for this change

Before version 0.45.0 of sqlparser, the special flag for both substring('xxxx', 1, 2) and substring('xxxx' from 1 for 2) was false.

After apache/datafusion-sqlparser-rs#1173, the special flag for substring('xxxx', 1, 2) becomes true.

We need to support both formats as before, so there's no need to check the special flag.

The 'from' and 'for' parameters don't support string types because they should be treated as regular expressions. Currently, substring is just an alias ofsubstr. This PR adds tests to verify this.

Reference document: https://www.postgresql.org/docs/current/functions-string.html

What changes are included in this PR?

Re-enable substring('xxxx', 1, 2).

Are these changes tested?

Yes

Are there any user-facing changes?

No

@github-actions github-actions bot added sql SQL Planner sqllogictest SQL Logic Tests (.slt) labels Apr 26, 2024
@jonahgao jonahgao marked this pull request as draft April 26, 2024 03:15
@jonahgao jonahgao marked this pull request as ready for review April 26, 2024 03:29
Comment on lines +1875 to +1883
query T
SELECT substring('alphabet', 1)
----
alphabet

query T
SELECT substring('alphabet', 3, 2)
----
ph
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

jonahgao and others added 3 commits April 26, 2024 20:47
Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
Copy link
Contributor

@Jefffrey Jefffrey left a comment

Choose a reason for hiding this comment

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

Looks good, nice catch 👍

@alamb
Copy link
Contributor

alamb commented Apr 26, 2024

Looks good to me @jonahgao -- thank you for the review @Jefffrey

@alamb alamb merged commit f8c623f into apache:main Apr 26, 2024
appletreeisyellow pushed a commit to influxdata/arrow-datafusion that referenced this pull request Apr 26, 2024
* fix: no longer support the `substring` function

* enable from-for format

* update test comment

* review feedback

* review feedback

Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>

---------

Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
@jonahgao jonahgao deleted the fix_substring branch April 26, 2024 22:13
appletreeisyellow pushed a commit to influxdata/arrow-datafusion that referenced this pull request Apr 30, 2024
* fix: no longer support the `substring` function

* enable from-for format

* update test comment

* review feedback

* review feedback

Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>

---------

Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
appletreeisyellow pushed a commit to influxdata/arrow-datafusion that referenced this pull request Apr 30, 2024
* fix: no longer support the `substring` function

* enable from-for format

* update test comment

* review feedback

* review feedback

Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>

---------

Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sql SQL Planner sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression: no longer support the substring function

3 participants