Skip to content

Commit

Permalink
Respect Hive statistics_enabled session in Glue metastore
Browse files Browse the repository at this point in the history
Before the change, the Hive Glue metastore wiring would respect the
`hive.table-statistics-enabled` configuration property, but not the
`statistics_enabled` session property.

After the change, handling of stats for Hive Glue metastore is same as
for other metastores. That means in particular that some of the
`DefaultGlueColumnStatisticsProvider` code will execute implicitly, as
not all stats-related operations are guarded by `statistics_enabled`
toggle today (see e.g. `SemiTransactionalHiveMetastore` and implicit
stats updates after table creation or alteration). This is considered
not a blocker.
  • Loading branch information
findepi committed Nov 14, 2023
1 parent b5238a0 commit 2fe1d20
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 110 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1585,6 +1585,8 @@ public ConnectorTableHandle beginStatisticsCollection(ConnectorSession session,
@Override
public void finishStatisticsCollection(ConnectorSession session, ConnectorTableHandle tableHandle, Collection<ComputedStatistics> computedStatistics)
{
verify(isStatisticsEnabled(session), "statistics not enabled");

HiveTableHandle handle = (HiveTableHandle) tableHandle;
SchemaTableName tableName = handle.getSchemaTableName();
Table table = metastore.getTable(tableName.getSchemaName(), tableName.getTableName())
Expand Down Expand Up @@ -3453,6 +3455,9 @@ public TableStatisticsMetadata getStatisticsCollectionMetadataForWrite(Connector
if (!isCollectColumnStatisticsOnWrite(session)) {
return TableStatisticsMetadata.empty();
}
if (!isStatisticsEnabled(session)) {
throw new TrinoException(NOT_SUPPORTED, "Table statistics must be enabled when column statistics collection on write is enabled");
}
if (isTransactional(tableMetadata.getProperties()).orElse(false)) {
// TODO(https://github.com/trinodb/trino/issues/1956) updating table statistics for transactional not supported right now.
return TableStatisticsMetadata.empty();
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.amazonaws.services.glue.model.Table;
import com.google.inject.Binder;
import com.google.inject.Key;
import com.google.inject.Module;
import com.google.inject.Provides;
import com.google.inject.Scopes;
import com.google.inject.Singleton;
Expand All @@ -42,7 +41,6 @@
import static com.google.inject.multibindings.Multibinder.newSetBinder;
import static com.google.inject.multibindings.OptionalBinder.newOptionalBinder;
import static io.airlift.concurrent.Threads.daemonThreadsNamed;
import static io.airlift.configuration.ConditionalModule.conditionalModule;
import static io.airlift.configuration.ConfigBinder.configBinder;
import static java.util.concurrent.Executors.newCachedThreadPool;
import static org.weakref.jmx.guice.ExportBinder.newExporter;
Expand Down Expand Up @@ -78,19 +76,8 @@ protected void setup(Binder binder)

binder.bind(Key.get(boolean.class, AllowHiveTableRename.class)).toInstance(false);

install(conditionalModule(
HiveConfig.class,
HiveConfig::isTableStatisticsEnabled,
getGlueStatisticsModule(DefaultGlueColumnStatisticsProviderFactory.class),
getGlueStatisticsModule(DisabledGlueColumnStatisticsProviderFactory.class)));
}

private Module getGlueStatisticsModule(Class<? extends GlueColumnStatisticsProviderFactory> statisticsProviderFactoryClass)
{
return internalBinder -> newOptionalBinder(internalBinder, GlueColumnStatisticsProviderFactory.class)
.setDefault()
.to(statisticsProviderFactoryClass)
.in(Scopes.SINGLETON);
newOptionalBinder(binder, GlueColumnStatisticsProviderFactory.class)
.setDefault().to(DefaultGlueColumnStatisticsProviderFactory.class).in(Scopes.SINGLETON);
}

@ProvidesIntoSet
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static io.trino.plugin.hive.HivePartition.UNPARTITIONED_ID;
import static io.trino.plugin.hive.HiveSessionProperties.isStatisticsEnabled;
import static java.util.Objects.requireNonNull;

public class MetastoreHiveStatisticsProvider
Expand All @@ -44,6 +45,9 @@ public MetastoreHiveStatisticsProvider(SemiTransactionalHiveMetastore metastore)
@Override
protected Map<String, PartitionStatistics> getPartitionsStatistics(ConnectorSession session, SchemaTableName table, List<HivePartition> hivePartitions, Set<String> columns)
{
if (!isStatisticsEnabled(session)) {
return ImmutableMap.of();
}
if (hivePartitions.isEmpty()) {
return ImmutableMap.of();
}
Expand Down

0 comments on commit 2fe1d20

Please sign in to comment.