-
Notifications
You must be signed in to change notification settings - Fork 141
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
#2550 Allowing joins when index names start with a period #2948
base: main
Are you sure you want to change the base?
Conversation
…period. Signed-off-by: Jeff <[email protected]>
|
||
assertThat( | ||
"identifier1.identifier2", equalTo(unquoteFullColumn("`identifier1`.`identifier2`"))); | ||
assertThat("identifier1.identifier2", equalTo(unquoteFullColumn("`identifier1`.identifier2"))); | ||
assertThat( | ||
".identifier1.identifier2", equalTo(unquoteFullColumn("`.identifier1`.`identifier2`"))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the queries (without join) work before this fixing?
select q.query_id
from .ubi_log_queries q
select q.query_id
from `.ubi_log_queries` q
Could you add some tests to verify the objective issue is fixed:
select q.query_id
from .ubi_log_queries q join .ubi_log_events e
on q.query_id = e.query_id
select q.query_id
from `.ubi_log_queries` q join `.ubi_log_events` e
on q.query_id = e.query_id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, those queries work fine.
I'm open to testing suggestions, but the issue is the presence of the period inside the backticks. When an index starts with a .
, the period is not included in the backticks.
if (text.startsWith(quote + ".")) { | ||
startsWithPeriod = true; | ||
text = quote + text.substring(2); | ||
} | ||
String[] strs = text.split("\\."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wonder if any easier way to do this. Essentially, we just want to split from the second dot if text starts with .
or `.
?
This PR is stalled because it has been open for 30 days with no activity. |
Description
Allows for join operations on indexes whose names start with periods.
Related Issues
Resolves #2550
Check List
--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.