You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/06/24 00:32:56 UTC
[GitHub] [iceberg] jfz opened a new pull request #2729: make iceberg sql command parsing more whitespace tolerant
jfz opened a new pull request #2729:
URL: https://github.com/apache/iceberg/pull/2729
Currently, iceberg specific sql command parsing is strict: extra white spaces between some of the keywords are not allowed.
This fix looses this restriction and make it more user friendly as well as more aligned with general sql syntax.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #2729: make iceberg sql command recognition more whitespace tolerant
Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #2729:
URL: https://github.com/apache/iceberg/pull/2729
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick edited a comment on pull request #2729: make iceberg sql command parsing more whitespace tolerant
Posted by GitBox <gi...@apache.org>.
kbendick edited a comment on pull request #2729:
URL: https://github.com/apache/iceberg/pull/2729#issuecomment-867259485
Is there any possibility that this would remove whitespace where it is intended?
Such as maybe specific formatting in table properties that are used internally, or specific formatting in table comments? I'm not sure how often people are using comments with specific formatting or tags with multiple meaningful whitespaces in a row in them, but throwing that out there for discussion.
EDIT: I see now that you're not rewriting the query, just for the purposes of checking if it's an iceberg query.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #2729: make iceberg sql command recognition more whitespace tolerant
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #2729:
URL: https://github.com/apache/iceberg/pull/2729#issuecomment-868675911
@jfz, thanks for pointing that out! I should have looked more closely.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] jfz commented on pull request #2729: make iceberg sql command recognition more whitespace tolerant
Posted by GitBox <gi...@apache.org>.
jfz commented on pull request #2729:
URL: https://github.com/apache/iceberg/pull/2729#issuecomment-867265389
> Is there any possibility that this would remove whitespace where it is intended?
>
> Such as maybe specific formatting in table properties that are used internally, or specific formatting in table comments? I'm not sure how often people are using comments with specific formatting or tags with multiple meaningful whitespaces in a row in them, but throwing that out there for discussion.
>
> EDIT: I see now that you're not rewriting the query, just for the purposes of checking if it's an iceberg query.
It doesn't rewrite the original query, I rephrased it from "parsing" to "recognition" to make it more clear.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on pull request #2729: make iceberg sql command recognition more whitespace tolerant
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on pull request #2729:
URL: https://github.com/apache/iceberg/pull/2729#issuecomment-868676899
Yep looks good to me, sorry I missed this yesterday
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #2729: make iceberg sql command recognition more whitespace tolerant
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #2729:
URL: https://github.com/apache/iceberg/pull/2729#issuecomment-868018053
@RussellSpitzer, can you take a look at this? I thought multiple spaces were allowed by the parser. Is this something we can configure in Antlr instead of in pre-processing?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #2729: make iceberg sql command parsing more whitespace tolerant
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #2729:
URL: https://github.com/apache/iceberg/pull/2729#issuecomment-867259485
Is there any possibility that this would remove whitespace where it is intended?
Such as maybe specific formatting in table properties that are used internally, or specific formatting in table comments? I'm not sure how often people are using comments with specific formatting or tags with multiple meaningful whitespaces in a row in them, but throwing that out there for discussion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] jfz commented on pull request #2729: make iceberg sql command recognition more whitespace tolerant
Posted by GitBox <gi...@apache.org>.
jfz commented on pull request #2729:
URL: https://github.com/apache/iceberg/pull/2729#issuecomment-868099048
> @RussellSpitzer, can you take a look at this? I thought multiple spaces were allowed by the parser. Is this something we can configure in Antlr instead of in pre-processing?
@rdblue this is only the check for deciding whether or not iceberg parser should be used; the actual iceberg antlr parser does handle white spaces properly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org