Skip to content

Conversation

@MitchellGale
Copy link
Contributor

Description

Adds support for escape characters \ to be used to escape characters in string literals. Keeps support for double quote to act as an escape if the surrounding quote is matching type (single quote, or double quote).

SELECT "Hello", 'Hello', "It""s", 'It''s', "It's", '"Its"', 'It\'s', 'It\\\'s', "\I\t\s"
fetched rows / total rows = 1/1
+-----------+-----------+-----------+-----------+----------+-----------+-----------+-------------+------------+
| "Hello"   | 'Hello'   | "It""s"   | 'It''s'   | "It's"   | '"Its"'   | 'It\'s'   | 'It\\\'s'   | "\I\t\s"   |
|-----------+-----------+-----------+-----------+----------+-----------+-----------+-------------+------------|
| Hello     | Hello     | It"s      | It's      | It's     | "Its"     | It's      | It\'s       | \I\t\s     |
+-----------+-----------+-----------+-----------+----------+-----------+-----------+-------------+------------+

Issues Resolved

#297

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

MitchellGale and others added 20 commits November 8, 2022 15:30
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Also removed unused StringUtils function

Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Escape character support for string literals
@MitchellGale MitchellGale requested a review from a team as a code owner December 22, 2022 01:11
@codecov-commenter
Copy link

codecov-commenter commented Dec 22, 2022

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.83%. Comparing base (18661b4) to head (16de714).
⚠️ Report is 811 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1206      +/-   ##
============================================
- Coverage     98.32%   95.83%   -2.49%     
  Complexity     3552     3552              
============================================
  Files           346      356      +10     
  Lines          8756     9414     +658     
  Branches        554      673     +119     
============================================
+ Hits           8609     9022     +413     
- Misses          142      334     +192     
- Partials          5       58      +53     
Flag Coverage Δ
query-workbench 62.76% <ø> (?)
sql-engine 98.32% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>
@dai-chen dai-chen added enhancement New feature or request backport 2.x labels Dec 23, 2022
@Yury-Fridlyand Yury-Fridlyand requested a review from penghuo January 4, 2023 01:38
@Yury-Fridlyand
Copy link
Collaborator

Isn't a breaking change?

@acarbonetto
Copy link
Collaborator

Isn't a breaking change?

Yes. This should not be a backport change.

@penghuo
Copy link
Collaborator

penghuo commented Jan 25, 2023

could you add more context about breacking change in PR. then during release stage, we can organice all the breaking changes in release notes.

@Yury-Fridlyand
Copy link
Collaborator

1. Add a test index:

curl -X POST "localhost:9200/dbg/_doc/?pretty" -H 'Content-Type: application/json' -d '{"someText": "\\1\\\\2\\\\\\3\\\\\\\\4\\\\\\\\\\5\\\\\\\\\\\\6"}'

(actually there are 1..6 backslashes, but they are escaped)

2. Run a query (without this fix):

opensearchsql> select * from dbg;
fetched rows / total rows = 1/1
+-----------------------------+
| someText                    |
|-----------------------------|
| \1\\2\\\3\\\\4\\\\\5\\\\\\6 |
+-----------------------------+
opensearchsql> select * from dbg where someText = "\1\\2\\\3\\\\4\\\\\5\\\\\\6";
fetched rows / total rows = 1/1
+-----------------------------+
| someText                    |
|-----------------------------|
| \1\\2\\\3\\\\4\\\\\5\\\\\\6 |
+-----------------------------+
opensearchsql> select * from dbg where someText = "\\1\\\\2\\\\\\3\\\\\\\\4\\\\\\\\\\5\\\\\\\\\\\\6";
fetched rows / total rows = 0/0
+------------+
| someText   |
|------------|
+------------+

3. Checkout this PR, build and install

4. Run the query again (with this fix):

opensearchsql> select * from dbg;
fetched rows / total rows = 1/1
+-----------------------------+
| someText                    |
|-----------------------------|
| \1\\2\\\3\\\\4\\\\\5\\\\\\6 |
+-----------------------------+
opensearchsql> select * from dbg where someText = "\1\\2\\\3\\\\4\\\\\5\\\\\\6";
fetched rows / total rows = 0/0
+------------+
| someText   |
|------------|
+------------+
opensearchsql> select * from dbg where someText = "\\1\\\\2\\\\\\3\\\\\\\\4\\\\\\\\\\5\\\\\\\\\\\\6";
fetched rows / total rows = 1/1
+-----------------------------+
| someText                    |
|-----------------------------|
| \1\\2\\\3\\\\4\\\\\5\\\\\\6 |
+-----------------------------+

5. Delete the index:

curl -XDELETE 'http://localhost:9200/dbg'

This PR affects how string literals processed only. Values are still returned as is, without processing escape sequences.

@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.19-dev failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/sql/backport-2.19-dev 2.19-dev
# Navigate to the new working tree
pushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-1206-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d6084125f4862464bbd6cb749b28082c542dbc3d
# Push it to GitHub
git push --set-upstream origin backport/backport-1206-to-2.19-dev
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/sql/backport-2.19-dev

Then, create a pull request where the base branch is 2.19-dev and the compare/head branch is backport/backport-1206-to-2.19-dev.

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.

8 participants