Skip to content

[CALCITE-6145] Function 'TRIM' without parameters throw NullPointerException#4359

Closed
xiedeyantu wants to merge 1 commit intoapache:mainfrom
xiedeyantu:origin/copy
Closed

[CALCITE-6145] Function 'TRIM' without parameters throw NullPointerException#4359
xiedeyantu wants to merge 1 commit intoapache:mainfrom
xiedeyantu:origin/copy

Conversation

@xiedeyantu
Copy link
Copy Markdown
Member

Only squash the #3869 and rebase main.

@mihaibudiu mihaibudiu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label May 4, 2025
"Invalid argument '': the length of the string describing "
+ "the trimmed character must be 1",
true);
f.checkFails("trim()", "Invalid number of arguments to function 'TRIM'. Was expecting at least 2 arguments", false);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This test cannot pass...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

then the PR is incomplete

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, give me some time and I'll see if I can handle it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Please remove the LGTM tag before I fix it.

@mihaibudiu mihaibudiu removed the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label May 4, 2025
@xiedeyantu
Copy link
Copy Markdown
Member Author

At present, errors will occur during the parsing stage. If TRIM cannot accept empty parameters, then this should be reasonable. I also reported parsing errors when verifying in MySQL and PGSQL. So does this PR still need for the current code?

Caused by: org.apache.calcite.sql.parser.SqlParseException: Encountered ")" at line 1, column 14.
Was expecting one of:
    "ABS" ...
    "ARRAY" ...
    "AVG" ...
    "BOTH" ...

@xuzifu666
Copy link
Copy Markdown
Member

So does this PR still need for the current code?

There are compatibility issues in syntax here which would block horoughly fix.

@mihaibudiu
Copy link
Copy Markdown
Contributor

See https://issues.apache.org/jira/browse/CALCITE-6708
and https://issues.apache.org/jira/browse/CALCITE-6709
So perhaps this has been fixed indirectly.

@xiedeyantu
Copy link
Copy Markdown
Member Author

See https://issues.apache.org/jira/browse/CALCITE-6708 and https://issues.apache.org/jira/browse/CALCITE-6709 So perhaps this has been fixed indirectly.

Yes, in my opinion, this PR can be closed now. WDYT?

@xiedeyantu
Copy link
Copy Markdown
Member Author

So does this PR still need for the current code?

There are compatibility issues in syntax here which would block horoughly fix.

Do we still need to fix this error? I think it's reasonable. What's your opinion?

@mihaibudiu
Copy link
Copy Markdown
Contributor

I will mark the issue resolved, you can close this PR

@xiedeyantu xiedeyantu closed this May 4, 2025
@xiedeyantu xiedeyantu deleted the origin/copy branch October 10, 2025 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants