[SPARK-50125] Add new error class - Data source does not support DML operation #48660
Conversation
| ], | ||
| "sqlState" : "KD010" | ||
| }, | ||
| "DATA_SOURCE_UNSUPPORTED_DML_OPERATION" : { |
There was a problem hiding this comment.
Maybe subclasses for each operation is better?
Operations are something like
'CREATE TABLE', 'ALTER NAMESPACE', etc
There was a problem hiding this comment.
It is common to name the operation: "ALTER TABLE', "UPDATE" as a parameter.
| "sqlState" : "KD011" | ||
| }, | ||
| "DATA_SOURCE_NOT_EXIST" : { | ||
| "message" : [ |
There was a problem hiding this comment.
I will add JIRA item when we are satisfied with the changes in PR
| ], | ||
| "sqlState" : "KD010" | ||
| }, | ||
| "DATA_SOURCE_UNSUPPORTED_DML_OPERATION" : { |
There was a problem hiding this comment.
If the feature is just not supported yet, we should consider to add a sub-class to UNSUPPORTED_FEATURE.
There was a problem hiding this comment.
This error class should be for data sources which does not have implementation for some DML operation, not like transient state.
| }, | ||
| "DATA_SOURCE_UNSUPPORTED_DML_OPERATION" : { | ||
| "message" : [ | ||
| "Data source '<provider>' does not support requested DML operation: '<operation>'" |
There was a problem hiding this comment.
Please, remove '' around . You should quote it by toSQLStmt when you form error message parameters.
There was a problem hiding this comment.
Even for provider as well? Or just for SQL operation?
There was a problem hiding this comment.
for the dml operation. Are you going to pass some SQL ops there, correct?
There was a problem hiding this comment.
Yes, some descriptive SQL operations, e.g. CREATE TABLE, DROP NAMESPACE, etc.
There was a problem hiding this comment.
Done, I also renamed operation to sqlOperation
There was a problem hiding this comment.
You need to propose a KD011, it seems new.
There was a problem hiding this comment.
| "Data source '<provider>' does not support requested DML operation: <sqlOperation>" | |
| "Data source '<provider>' does not support requested operation: <sqlOperation>" |
We can be more generic.
urosstan-db
force-pushed
the
add-new-error-condition
branch
from
3d34c2d to
950ea4b
Compare
urosstan-db
changed the title
MaxGekk
left a comment
MaxGekk
left a comment
There was a problem hiding this comment.
@urosstan-db Are you going to raise the error from somewhere?
| ], | ||
| "sqlState" : "42K03" | ||
| }, | ||
| "DATA_SOURCE_UNSUPPORTED_DML_OPERATION" : { |
There was a problem hiding this comment.
if we removed DML from message, let's remove it from the condition name.
There was a problem hiding this comment.
I want to have it just exception dedicated to DML operations, if it is needed I can revert DML in message? Serge suggested me to do that
There was a problem hiding this comment.
The message should be in sync with the name. So yes, I implied to remove it from both.
Co-authored-by: Maxim Gekk <max.gekk@gmail.com>
No for now, not in Spark to be more precise. |
Co-authored-by: Serge Rielau <srielau@users.noreply.github.com>
In that case, I don't think it should be merged to OSS Spark, sorry. |
Ok thanks, I thought to avoid introducing of multiple similar codes, but let's close this one then. |
3 participants
What changes were proposed in this pull request?
Add new error class which indicates certain data source does not support DML operation.
Why are the changes needed?
Sometimes, Data Source V2 does not support DML operations such as create table, alter table, drop namespace, etc i.e. it is read only data source, so we need some new error class which will be thrown in case we implement such data source.
Does this PR introduce any user-facing change?
No
How was this patch tested?
No testing needed, compile only
Was this patch authored or co-authored using generative AI tooling?
No