kasakrisz commented on code in PR #6449:
URL: https://github.com/apache/hive/pull/6449#discussion_r3324461582
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewOperation.java:
##########
@@ -105,6 +114,25 @@ public int execute() throws HiveException {
return 0;
}
+ private void pushExternalLogicalViewSessionHints(boolean replace, boolean
ifNotExists) {
+ SessionStateUtil.addResource(context.getConf(),
Constants.EXTERNAL_LOGICAL_VIEW_DDL_REPLACE,
+ Boolean.toString(replace));
+ SessionStateUtil.addResource(context.getConf(),
Constants.EXTERNAL_LOGICAL_VIEW_CREATE_IF_NOT_EXISTS,
+ Boolean.toString(ifNotExists));
+ }
Review Comment:
The values of `replace` and `ifNotExists` come from session are not used in
case of `HiveIcebergMetaHook`:
1. When view does not exist it always goes to
`HiveIcebergMetaHook.commitCreateTable`. The flags' values `replace` and
`ifNotExists` doesn't matter the view has to be created.
2. When view exists and `ifNotExists` is `true` the operation is aborted
here:
https://github.com/kasakrisz/hive/blob/1ba1712e9e5a32534aedcc70a2b66843b6d71421/ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewOperation.java#L65-L68
3. When view exists and `replace` is `true` alter table and
`HiveIcebergMetaHook.commitAlterTable` is called
4. When view exists and both `replace` and `ifNotExists` are `false` then we
throw View already exists here
https://github.com/kasakrisz/hive/blob/1ba1712e9e5a32534aedcc70a2b66843b6d71421/ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewOperation.java#L71
5. When view does not exist and both `replace` and `ifNotExists` are `true`
we throw exception because only one of them is allowed at a time
https://github.com/kasakrisz/hive/blob/1ba1712e9e5a32534aedcc70a2b66843b6d71421/ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewAnalyzer.java#L80-L81
I haven't found any tests for replace and if-not-exists in case of
`HiveRESTCatalogClient`. Please add some. I expect that these flags are handled
by `CreateViewOperation` in this case too and don't need to be added to the
session state.
[difin] - couldn't reply on this comment. Addressed, please see
https://github.com/apache/hive/pull/6449#discussion_r3327175743
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
