HIVE-28642:column stats state unnecessarily created when hive.stats.c… #5560
Conversation
…olumn.autogather=false
|
The patch seems reasonable, but I have a little doubt about the behaviour of If I understand correctly, Also I found that |
|
thanks @ngsg for the review. I changed CreateTableLikeOperation, AbstractAddPartitionAnalyzer and the loadPartition. for loadPartition the actual column stats is updated later. for CreateTableLikeOperation the call should not have column stats so setting state true is wrong. i can't find anywhere using the COLUMN_STATS_ACCURATE to trigger compute, it seems it is just used for parsing. some user can not change their metastore db so backport HIVE-20221 not an option for them and instead they want to just turn column stats off. |
asf-ci-hive
added
tests unstable
tests pending
and removed
tests pending
tests unstable
labels
| @@ -74,8 +74,7 @@ public int execute() throws HiveException { | |||
|
|
|||
| if (desc.getLocation() == null && !tbl.isPartitioned() && | |||
| context.getConf().getBoolVar(HiveConf.ConfVars.HIVE_STATS_AUTOGATHER)) { | |||
| StatsSetupConst.setStatsStateForCreateTable(tbl.getTTable().getParameters(), | |||
| MetaStoreUtils.getColumnNames(tbl.getCols()), StatsSetupConst.TRUE); | |||
| StatsSetupConst.setStatsStateForCreateTable(tbl.getTTable().getParameters(),null, StatsSetupConst.TRUE); | |||
There was a problem hiding this comment.
NIT, we missed a space between tbl.getTTable().getParameters() and null.
| MetaStoreUtils.getColumnNames(table.getCols()), StatsSetupConst.TRUE); | ||
| List<String> colNames = conf.getBoolVar(HiveConf.ConfVars.HIVE_STATS_COL_AUTOGATHER) ? | ||
| MetaStoreUtils.getColumnNames(table.getCols()) : null; | ||
| StatsSetupConst.setStatsStateForCreateTable(params,colNames, StatsSetupConst.TRUE); |
There was a problem hiding this comment.
NIT, the same space issue here.
| @@ -2820,8 +2820,7 @@ private Partition loadPartitionInternal(Path loadPath, Table tbl, Map<String, St | |||
| if (oldPart == null) { | |||
| newTPart.getTPartition().setParameters(new HashMap<String,String>()); | |||
| if (this.getConf().getBoolVar(HiveConf.ConfVars.HIVE_STATS_AUTOGATHER)) { | |||
| StatsSetupConst.setStatsStateForCreateTable(newTPart.getParameters(), | |||
| MetaStoreUtils.getColumnNames(tbl.getCols()), StatsSetupConst.TRUE); | |||
| StatsSetupConst.setStatsStateForCreateTable(newTPart.getParameters(),null, StatsSetupConst.TRUE); | |||
There was a problem hiding this comment.
NIT, the same space issue here, too.
|
Could you check the result of Hive CI? It seems that your last change disables |
|
|
LGTM with one nit and updating 3 qfile outputs I have checked that 3 qfiles failed because Hive marks all columns as accurate in I wonder if there exists any incorrect column statistics computation issues which this patch can fix, but I couldn't find such problems so far. The following link contains a query that I used to investigate. It would be appreciated if anyone know and could share an example of incorrect column statistics computation. |
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import java.util.*; |
There was a problem hiding this comment.
NIT: Could we remove star-import? Star-import should be used in limited situations.
|
I wonder why we don't have to apply a similar update on the other classes using |
do not set column stats state when load new partition if hive.stats.column.autogather=false
What changes were proposed in this pull request?
simplify query string when load new partition by removing the column stats state setup when HIVE_STATS_COL_AUTOGATHER=false
Why are the changes needed?
avoid PARAM_VALUE too long error from metastore db.
Does this PR introduce any user-facing change?
no
Is the change a dependency upgrade?
no
How was this patch tested?
tested locally