Skip to content

[CALCITE-7456] Enable the TRY_CAST function to support the MSSQL dialect#4856

Open
cjj2010 wants to merge 3 commits intoapache:mainfrom
cjj2010:CALCITE-7456
Open

[CALCITE-7456] Enable the TRY_CAST function to support the MSSQL dialect#4856
cjj2010 wants to merge 3 commits intoapache:mainfrom
cjj2010:CALCITE-7456

Conversation

@cjj2010
Copy link
Copy Markdown

@cjj2010 cjj2010 commented Mar 30, 2026

@xiedeyantu
Copy link
Copy Markdown
Member

CI seems unhappy. The commit message misses the prefix CALCITE-XXXX.

@cjj2010
Copy link
Copy Markdown
Author

cjj2010 commented Mar 31, 2026

CI seems unhappy. The commit message misses the prefix CALCITE-XXXX.

thanks, done

@cjj2010 cjj2010 changed the title [CALCITE-7456] Improve the conversion of try_cast function with mssql [CALCITE-7456] Enable the TRY_CAST function to support the MSSQL dialect Mar 31, 2026
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 1, 2026

Copy link
Copy Markdown
Member

@xuzifu666 xuzifu666 left a comment

Choose a reason for hiding this comment

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

If it has been thoroughly tested and verified in MSSQL, I think it's OK.

Copy link
Copy Markdown
Member

@caicancai caicancai left a comment

Choose a reason for hiding this comment

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

I feel the implementation is a bit strange; I'll take a look at the code and verify it tomorrow.

final SqlWriter.Frame frame = writer.startFunCall("CEILING");
call.operand(0).unparse(writer, leftPrec, rightPrec);
writer.endFunCall(frame);
} else if (call.getKind() == SqlKind.SAFE_CAST) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Manually assembling unparsed logic is repetitive and fragile.

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.

4 participants