HIVE-20057: Fix Hive table conversion DESCRIBE table bug

[animenon]

Fix for #HIVE-20057

Issue: Table Type wrongly shown as MANAGED_TABLE after converting table from MANAGED to EXTERNAL using ALTER TABLE t SET TBLPROPERTIES ('EXTERNAL'='True')

(this is shown correctly only for 'EXTERNAL'='TRUE)

[rmsmani]

HI
Create the git diff file and apply in JIRA Ticket, so that the PRE-COMMITS testing will be done automatically.

[rmsmani]

Merge the source with your local branch and raise the pull request again, to resolve the conflicting issues.

[animenon]

@rmsmani done.

[rmsmani]

+1

[rmsmani]

Hi @ashutosh-bapat, @sankarh
Can you merge the code

[sankarh]

I think, this issue doesn't exist in master code.
We use Boolean.parseBoolean to read the "EXTERNAL" property which ignores case. Also, the fix in the patch is to use equalsIgnoreCase when compare the TableType and not "EXTERNAL" property which is irrelevant.
If I misunderstood your scenario, please let me know.
Also, it would be better to add a unit test to reproduce this bug in master code.

[animenon]

@sankarh The table type gets converted as expected but the describe formatted view shows it wrongly.
Example: I converted the below Managed table to External using .. SET TBLPROPERTIES ('EXTERNAL'='True')

If I do the same using .. SET TBLPROPERTIES ('EXTERNAL'='TRUE')

Let me know if you need further details.

[sankarh]

@animenon
I tested it myself and it worked fine for me. Probably, you are using different version of Hive or Java.
Take a look at this code (apache/master) which converts managed to external type.
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java#L4092
It parses EXTERNAL property using Boolean.parseBoolean which ignores case and hence converts to EXTERNAL_TABLE type.

[sankarh]

Even, if it behaves differently for different cases of EXTERNAL property, still I'm not sure how your fix would solve the issue as you are ignoring case of TableType but not EXTERNAL property. Pls explain.

[animenon]

IgnoreCase used here :
if (TableType.EXTERNAL_TABLE.toString().equalsIgnoreCase(tableType))
for EXTERNAL property and tableType equals check would check if equals ignoring the case of letters.

I agree the tableType ignoreCase here may not be required.

[animenon]

Yes the Boolean.parseBoolean would ignore case and give correct isExternal value, but the thing is you wouldn't reach there..

TableType.EXTERNAL_TABLE equals tableType is checked before if (!isExternal) that would fail. ref

Like you pointed out rightly, isExternal ignores case but the flow wouldn't goto that condition because the condition before that fails.

[sankarh]

@animenon
Thanks for thr reply! But, I'm still confused.
As per your point, isExternal is proper (both "True" and "TRUE"). But, why would tableType have different case based on input for EXTERNAL property?
Below is the code that reads EXTERNAL property from ALTER TABLE command and sets correct tableType.
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java#L4092

String externalProp = alterTbl.getProps().get("EXTERNAL");
      if (externalProp != null) {
        if (Boolean.parseBoolean(externalProp) && tbl.getTableType() == TableType.MANAGED_TABLE) {
          tbl.setTableType(TableType.EXTERNAL_TABLE);
        } else if (!Boolean.parseBoolean(externalProp) && tbl.getTableType() == TableType.EXTERNAL_TABLE) {
          tbl.setTableType(TableType.MANAGED_TABLE);
        }
      }

If you notice, we use Boolean.parseBoolean(externalProp) here which ignores case.
I didn't get how two values of externalProp ("True" and "TRUE") influences the table type set via setTableType.
Pls point to what I'm missing here.

@rmsmani
Please check if I miss anything.

[rmsmani]

@sankarh
How about changing the condition to equals in the below lines (both if & else condition)
if (Boolean.parseBoolean(externalProp) && tbl.getTableType() == TableType.MANAGED_TABLE) {

[sankarh]

@rmsmani and @animenon
HIVE-16324 and HIVE-19753 have already fixed this bug in apache/master. So, this bug doesn't exist anymore.
Probably, you might see it in older Hive versions which doesn't include these patches.
So, please close this jira as invalid.
If you want to push this patch in different branch where the bug exists, then pls let me know.

[animenon]

@sankarh - For the newer version, HIVE-16324 seems to fix it, the version of hive I am using still has this issue, will check my hive version update in appropriate branch.
Thanks for checking on this.

[animenon]

Handled in HIVE-16324 for Hive v3, will re-raise patch for appropriate version.
Thanks @rmsmani and @sankarh .