hive: support for special not expression `!a` and raise error for `a!` factorial operator #1472

wugeer · 2024-10-12T07:38:17Z

Closes: #1471
Closes: #1473

coveralls · 2024-10-16T16:30:01Z

Pull Request Test Coverage Report for Build 11303950713

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

28 of 38 (73.68%) changed or added relevant lines in 5 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.02%) to 89.298%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/dialect/mod.rs	1	2	50.0%
src/dialect/postgresql.rs	1	2	50.0%
tests/sqlparser_hive.rs	13	15	86.67%
src/parser/mod.rs	12	18	66.67%

Totals
Change from base Build 11294882452:	-0.02%
Covered Lines:	30172
Relevant Lines:	33788

💛 - Coveralls

src/dialect/mod.rs

src/parser/mod.rs

src/ast/operator.rs

iffyio · 2024-10-18T13:18:34Z

src/parser/mod.rs

@@ -1174,6 +1175,14 @@ impl<'a> Parser<'a> {
                    ),
                })
            }
+            Token::ExclamationMark if !self.dialect.supports_factorial_operator() => {


Suggested change

Token::ExclamationMark if !self.dialect.supports_factorial_operator() => {

Token::ExclamationMark if self.dialect.supports_bang_not_operator() => {

See comment a bit lower for rationale to have a dedicated api for this behavior

Similarly, this comment looks unadressed? The new feature ideally has its own dialect method. Otherwise it can get complicated to try to infer when a feature should take precedence

The code has been re-pushed.

tests/sqlparser_hive.rs

wugeer · 2024-10-25T14:35:48Z

@iffyio Thank you for your suggestions—I learned a lot from them， I've resubmitted an updated version of the code. When you have a moment, could you please review it? Thanks!:)

src/parser/mod.rs

src/ast/operator.rs

iffyio · 2024-10-27T06:19:42Z

src/parser/mod.rs

@@ -1174,6 +1175,14 @@ impl<'a> Parser<'a> {
                    ),
                })
            }
+            Token::ExclamationMark if !self.dialect.supports_factorial_operator() => {


Similarly, this comment looks unadressed? The new feature ideally has its own dialect method. Otherwise it can get complicated to try to infer when a feature should take precedence

src/parser/mod.rs

tests/sqlparser_hive.rs

…r a! factorial operator in Hive dialect

wugeer · 2024-10-27T11:14:54Z

@iffyio Sorry, my oversight caused the code not to be merged. The code has been re-pushed.

wugeer force-pushed the fix_issue_1471 branch from c0fac44 to ab78bc2 Compare October 12, 2024 07:47

iffyio reviewed Oct 18, 2024

View reviewed changes

wugeer force-pushed the fix_issue_1471 branch from db466d2 to 2196dc1 Compare October 25, 2024 14:32

wugeer requested a review from iffyio October 25, 2024 14:35

iffyio reviewed Oct 27, 2024

View reviewed changes

wugeer requested a review from iffyio October 27, 2024 08:17

wugeer force-pushed the fix_issue_1471 branch 2 times, most recently from de68644 to 99910dd Compare October 27, 2024 11:05

feat: add support for special bang not operator !a and raise error fo…

99910dd

…r a! factorial operator in Hive dialect

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hive: support for special not expression `!a` and raise error for `a!` factorial operator #1472

hive: support for special not expression `!a` and raise error for `a!` factorial operator #1472

wugeer commented Oct 12, 2024 •

edited

Loading

coveralls commented Oct 16, 2024

iffyio Oct 18, 2024

iffyio Oct 27, 2024

wugeer Oct 27, 2024

wugeer commented Oct 25, 2024

iffyio Oct 27, 2024

wugeer commented Oct 27, 2024

	Token::ExclamationMark if !self.dialect.supports_factorial_operator() => {
	Token::ExclamationMark if self.dialect.supports_bang_not_operator() => {

hive: support for special not expression !a and raise error for a! factorial operator #1472

Are you sure you want to change the base?

hive: support for special not expression !a and raise error for a! factorial operator #1472

Conversation

wugeer commented Oct 12, 2024 • edited Loading

coveralls commented Oct 16, 2024

Pull Request Test Coverage Report for Build 11303950713

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

iffyio Oct 18, 2024

Choose a reason for hiding this comment

iffyio Oct 27, 2024

Choose a reason for hiding this comment

wugeer Oct 27, 2024

Choose a reason for hiding this comment

wugeer commented Oct 25, 2024

iffyio Oct 27, 2024

Choose a reason for hiding this comment

wugeer commented Oct 27, 2024

hive: support for special not expression `!a` and raise error for `a!` factorial operator #1472

hive: support for special not expression `!a` and raise error for `a!` factorial operator #1472

wugeer commented Oct 12, 2024 •

edited

Loading