[HUDI-7213] When using wrong tabe.type value in hudi catalog happends npe #10300
Conversation
| String sparkTableType = VALUE_MAPPING.get(e.getValue()); | ||
| if (sparkTableType == null) { | ||
| throw new HoodieValidationException(String.format("%s's value is invalid", e.getKey())); | ||
| } |
There was a problem hiding this comment.
Nice catch, can you write a UT in TestHoodieCatalog ?
|
@danny0405 hello, i have added a ut in TestHoodieHiveCatalog,as the npe appears in translateFlinkTableProperties2Spark |
| try { | ||
| hoodieCatalog.createTable(tablePath, table, false); | ||
| } catch (HoodieValidationException e) { | ||
| assertEquals("table.type's value is invalid", e.getMessage()); |
There was a problem hiding this comment.
This patch fixes the hoodie catalog instead of hoodie hive catalog right?
There was a problem hiding this comment.
this patch fix the hoodie hive catalog, when using hudi hive catalog ,using create if not exists table , the code will go to org.apache.hudi.table.catalog.HoodieHiveCatalog#createTable | org.apache.hudi.table.catalog.HoodieHiveCatalog#instantiateHiveTable| org.apache.hudi.table.catalog.TableOptionProperties#translateFlinkTableProperties2Spark
and in this method if the table.type is wrong it will throw npe
There was a problem hiding this comment.
Can we use assertThrows instead?
| @@ -477,6 +478,8 @@ public void createTable(ObjectPath tablePath, CatalogBaseTable table, boolean ig | |||
| if (!ignoreIfExists) { | |||
| throw new TableAlreadyExistException(getName(), tablePath, e); | |||
| } | |||
| } catch (HoodieValidationException e) { | |||
| throw e; | |||
| } catch (Exception e) { | |||
| throw new HoodieCatalogException( | |||
There was a problem hiding this comment.
Shouldn't any exception been caught by the Exception ?
There was a problem hiding this comment.
@danny0405 yes, but I throws a HoodieValidationException, maybe i need to fix ut to assert throw a HoodieCatalogException
There was a problem hiding this comment.
maybe i need to fix ut to assert throw a HoodieCatalogException
You should.
Change Logs
when using wrong table.type in hudi catalog, then the VALUE_MAPPING.get() will return null and throw npe, as detail in jira
maybe we need to tell user the wrong key
Impact
Describe any public API or user-facing feature change or any performance impact.
Risk level (write none, low medium or high below)
If medium or high, explain what verification was done to mitigate the risks.
Documentation Update
Describe any necessary documentation update if there is any new feature, config, or user-facing change
ticket number here and follow the instruction to make
changes to the website.
Contributor's checklist