Skip to content

Commit 1b8d043

Browse files
wraymowraymo
committed
1. use singleton for ClpPlanOptimizerProvider
2. rename splitPath to path in ClpSplit 3. add more properties for equals, hashCode, and toString in ClpTableLayoutHandle 4. remove public from methods in ClpMetadataProvider 5. change most fo log.error to log.warn in ClpMySqlSplitProvider and ClpMySqlMetadataProvider and fix one logging statement in ClpMySqlMetadataProvider 6. fix two minor issues in ClpMetadata 7. throw an exception in getRecordSet 8. improve TestClpOptimizer 9. refactor ClpSchemaTree and fix several bugs 10. add StorageType to ClpTableHandle 11. add archive_storage_type to ClpMySqlMetadataProvider and ClpMySqlSplitProvider 12. change listing table names to listing ClpTableHandle in ClpMetadataProvider reflect the changes in ClpMetadata 13. use constants in ClpMySqlMetadataProvider and ClpMySqlSplitProvider 14. update TestClpSplit and TestClpMetadata to include archive_storage_type 15. remove archive-source from ClpConfig and docs 16. fix legacy configuration names in clp docs 17. quote table names and column names in ClpMySqlMetadataProvider and ClpMySqlSplitProvider 18. add tablePath to ClpTableHandle and simplifies the logic in ClpMySqlSplitProvider
1 parent 550e448 commit 1b8d043

File tree

19 files changed

+219
-171
lines changed

19 files changed

+219
-171
lines changed

‎presto-clp/pom.xml‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,6 @@
4949
<artifactId>guice</artifactId>
5050
</dependency>
5151

52-
<dependency>
53-
<groupId>com.google.code.findbugs</groupId>
54-
<artifactId>jsr305</artifactId>
55-
<optional>true</optional>
56-
</dependency>
57-
5852
<dependency>
5953
<groupId>com.google.guava</groupId>
6054
<artifactId>guava</artifactId>

‎presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpColumnHandle.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public ColumnMetadata getColumnMetadata()
8585
@Override
8686
public int hashCode()
8787
{
88-
return Objects.hash(columnName, columnType);
88+
return Objects.hash(columnName, originalColumnName, columnType, nullable);
8989
}
9090

9191
@Override
@@ -99,14 +99,17 @@ public boolean equals(Object obj)
9999
}
100100
ClpColumnHandle other = (ClpColumnHandle) obj;
101101
return Objects.equals(this.columnName, other.columnName) &&
102-
Objects.equals(this.columnType, other.columnType);
102+
Objects.equals(this.originalColumnName, other.originalColumnName) &&
103+
Objects.equals(this.columnType, other.columnType) &&
104+
Objects.equals(this.nullable, other.nullable);
103105
}
104106

105107
@Override
106108
public String toString()
107109
{
108110
return toStringHelper(this)
109111
.add("columnName", columnName)
112+
.add("originalColumnName", originalColumnName)
110113
.add("columnType", columnType)
111114
.add("nullable", nullable)
112115
.toString();

‎presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpConfig.java‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,6 @@
2020

2121
public class ClpConfig
2222
{
23-
public enum ArchiveSource
24-
{
25-
LOCAL,
26-
S3
27-
}
28-
2923
public enum MetadataProviderType
3024
{
3125
MYSQL
@@ -45,7 +39,6 @@ public enum SplitProviderType
4539
private String metadataTablePrefix;
4640
private long metadataRefreshInterval = 60;
4741
private long metadataExpireInterval = 600;
48-
private ArchiveSource archiveSource = ArchiveSource.LOCAL;
4942
private SplitProviderType splitProviderType = SplitProviderType.MYSQL;
5043

5144
public static final Pattern SAFE_SQL_IDENTIFIER = Pattern.compile("^[a-zA-Z0-9_]+$");
@@ -163,18 +156,6 @@ public ClpConfig setMetadataExpireInterval(long metadataExpireInterval)
163156
return this;
164157
}
165158

166-
public ArchiveSource getArchiveSource()
167-
{
168-
return archiveSource;
169-
}
170-
171-
@Config("clp.archive-source")
172-
public ClpConfig setArchiveSource(ArchiveSource archiveSource)
173-
{
174-
this.archiveSource = archiveSource;
175-
return this;
176-
}
177-
178159
public SplitProviderType getSplitProviderType()
179160
{
180161
return splitProviderType;

‎presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpConnector.java‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
import com.facebook.presto.spi.connector.ConnectorRecordSetProvider;
2222
import com.facebook.presto.spi.connector.ConnectorSplitManager;
2323
import com.facebook.presto.spi.connector.ConnectorTransactionHandle;
24-
import com.facebook.presto.spi.function.FunctionMetadataManager;
25-
import com.facebook.presto.spi.function.StandardFunctionResolution;
2624
import com.facebook.presto.spi.transaction.IsolationLevel;
2725

2826
import javax.inject.Inject;
@@ -36,31 +34,27 @@ public class ClpConnector
3634

3735
private final LifeCycleManager lifeCycleManager;
3836
private final ClpMetadata metadata;
37+
private final ClpPlanOptimizerProvider planOptimizerProvider;
3938
private final ClpRecordSetProvider recordSetProvider;
4039
private final ClpSplitManager splitManager;
41-
private final FunctionMetadataManager functionManager;
42-
private final StandardFunctionResolution functionResolution;
43-
4440
@Inject
4541
public ClpConnector(LifeCycleManager lifeCycleManager,
4642
ClpMetadata metadata,
43+
ClpPlanOptimizerProvider planOptimizerProvider,
4744
ClpRecordSetProvider recordSetProvider,
48-
ClpSplitManager splitManager,
49-
FunctionMetadataManager functionManager,
50-
StandardFunctionResolution functionResolution)
45+
ClpSplitManager splitManager)
5146
{
5247
this.lifeCycleManager = requireNonNull(lifeCycleManager, "lifeCycleManager is null");
5348
this.metadata = requireNonNull(metadata, "metadata is null");
49+
this.planOptimizerProvider = requireNonNull(planOptimizerProvider, "planOptimizerProvider is null");
5450
this.recordSetProvider = requireNonNull(recordSetProvider, "recordSetProvider is null");
5551
this.splitManager = requireNonNull(splitManager, "splitManager is null");
56-
this.functionManager = requireNonNull(functionManager, "functionManager is null");
57-
this.functionResolution = requireNonNull(functionResolution, "functionResolution is null");
5852
}
5953

6054
@Override
6155
public ConnectorPlanOptimizerProvider getConnectorPlanOptimizerProvider()
6256
{
63-
return new ClpPlanOptimizerProvider(functionManager, functionResolution);
57+
return planOptimizerProvider;
6458
}
6559

6660
@Override

‎presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpMetadata.java‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public class ClpMetadata
4949
private static final String DEFAULT_SCHEMA_NAME = "default";
5050
private final ClpMetadataProvider clpMetadataProvider;
5151
private final LoadingCache<SchemaTableName, List<ClpColumnHandle>> columnHandleCache;
52-
private final LoadingCache<String, List<String>> tableNameCache;
52+
private final LoadingCache<String, List<ClpTableHandle>> tableHandleCache;
5353

5454
@Inject
5555
public ClpMetadata(ClpConfig clpConfig, ClpMetadataProvider clpMetadataProvider)
@@ -58,10 +58,10 @@ public ClpMetadata(ClpConfig clpConfig, ClpMetadataProvider clpMetadataProvider)
5858
.expireAfterWrite(clpConfig.getMetadataExpireInterval(), SECONDS)
5959
.refreshAfterWrite(clpConfig.getMetadataRefreshInterval(), SECONDS)
6060
.build(CacheLoader.from(this::loadColumnHandles));
61-
this.tableNameCache = CacheBuilder.newBuilder()
61+
this.tableHandleCache = CacheBuilder.newBuilder()
6262
.expireAfterWrite(clpConfig.getMetadataExpireInterval(), SECONDS)
6363
.refreshAfterWrite(clpConfig.getMetadataRefreshInterval(), SECONDS)
64-
.build(CacheLoader.from(this::loadTableNames));
64+
.build(CacheLoader.from(this::loadTableHandles));
6565

6666
this.clpMetadataProvider = clpMetadataProvider;
6767
}
@@ -71,14 +71,14 @@ private List<ClpColumnHandle> loadColumnHandles(SchemaTableName schemaTableName)
7171
return clpMetadataProvider.listColumnHandles(schemaTableName);
7272
}
7373

74-
private List<String> loadTableNames(String schemaName)
74+
private List<ClpTableHandle> loadTableHandles(String schemaName)
7575
{
76-
return clpMetadataProvider.listTableNames(schemaName);
76+
return clpMetadataProvider.listTableHandles(schemaName);
7777
}
7878

79-
private List<String> listTables(String schemaName)
79+
private List<ClpTableHandle> listTables(String schemaName)
8080
{
81-
return tableNameCache.getUnchecked(schemaName);
81+
return tableHandleCache.getUnchecked(schemaName);
8282
}
8383

8484
private List<ClpColumnHandle> listColumns(SchemaTableName schemaTableName)
@@ -101,7 +101,7 @@ public List<SchemaTableName> listTables(ConnectorSession session, Optional<Strin
101101
}
102102

103103
return listTables(schemaNameValue).stream()
104-
.map(tableName -> new SchemaTableName(schemaNameValue, tableName))
104+
.map(tableHandle -> new SchemaTableName(schemaNameValue, tableHandle.getSchemaTableName().getTableName()))
105105
.collect(ImmutableList.toImmutableList());
106106
}
107107

@@ -113,11 +113,10 @@ public ConnectorTableHandle getTableHandle(ConnectorSession session, SchemaTable
113113
return null;
114114
}
115115

116-
if (!listTables(schemaName).contains(tableName.getTableName())) {
117-
return null;
118-
}
119-
120-
return new ClpTableHandle(tableName);
116+
return listTables(schemaName).stream()
117+
.filter(tableHandle -> tableHandle.getSchemaTableName().equals(tableName))
118+
.findFirst()
119+
.orElse(null);
121120
}
122121

123122
@Override
@@ -155,13 +154,13 @@ public Map<SchemaTableName, List<ColumnMetadata>> listTableColumns(ConnectorSess
155154
{
156155
requireNonNull(prefix, "prefix is null");
157156
String schemaName = prefix.getSchemaName();
158-
if (schemaName != null && !schemaName.equals(DEFAULT_SCHEMA_NAME)) {
157+
if (schemaName != null && !listSchemaNames(session).contains(schemaName)) {
159158
return ImmutableMap.of();
160159
}
161160

162161
List<SchemaTableName> schemaTableNames;
163162
if (prefix.getTableName() == null) {
164-
schemaTableNames = listTables(session, Optional.of(prefix.getSchemaName()));
163+
schemaTableNames = listTables(session, Optional.ofNullable(prefix.getSchemaName()));
165164
}
166165
else {
167166
schemaTableNames = ImmutableList.of(new SchemaTableName(prefix.getSchemaName(), prefix.getTableName()));

‎presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpModule.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ protected void setup(Binder binder)
3232
{
3333
binder.bind(ClpConnector.class).in(Scopes.SINGLETON);
3434
binder.bind(ClpMetadata.class).in(Scopes.SINGLETON);
35+
binder.bind(ClpPlanOptimizerProvider.class).in(Scopes.SINGLETON);
3536
binder.bind(ClpRecordSetProvider.class).in(Scopes.SINGLETON);
3637
binder.bind(ClpSplitManager.class).in(Scopes.SINGLETON);
3738
configBinder(binder).bindConfig(ClpConfig.class);

‎presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpRecordSetProvider.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,6 @@ public RecordSet getRecordSet(ConnectorTransactionHandle transactionHandle,
3131
ConnectorSplit split,
3232
List<? extends ColumnHandle> columns)
3333
{
34-
return null;
34+
throw new UnsupportedOperationException("getRecordSet is not supported");
3535
}
3636
}

‎presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpSplit.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,27 +21,25 @@
2121
import com.fasterxml.jackson.annotation.JsonProperty;
2222
import com.google.common.collect.ImmutableList;
2323

24-
import javax.annotation.Nullable;
25-
2624
import java.util.List;
2725

2826
import static com.facebook.presto.spi.schedule.NodeSelectionStrategy.NO_PREFERENCE;
2927

3028
public class ClpSplit
3129
implements ConnectorSplit
3230
{
33-
private final String splitPath;
31+
private final String path;
3432

3533
@JsonCreator
36-
public ClpSplit(@JsonProperty("splitPath") @Nullable String splitPath)
34+
public ClpSplit(@JsonProperty("path") String path)
3735
{
38-
this.splitPath = splitPath;
36+
this.path = path;
3937
}
4038

4139
@JsonProperty
42-
public String getSplitPath()
40+
public String getPath()
4341
{
44-
return splitPath;
42+
return path;
4543
}
4644

4745
@Override

‎presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpTableHandle.java‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,29 @@
2020

2121
import java.util.Objects;
2222

23+
import static com.google.common.base.MoreObjects.toStringHelper;
24+
2325
public class ClpTableHandle
2426
implements ConnectorTableHandle
2527
{
28+
public enum StorageType
29+
{
30+
FS, // Local File System
31+
S3
32+
}
33+
2634
private final SchemaTableName schemaTableName;
35+
private final String tablePath;
36+
private final StorageType storageType;
2737

2838
@JsonCreator
29-
public ClpTableHandle(@JsonProperty("schemaTableName") SchemaTableName schemaTableName)
39+
public ClpTableHandle(@JsonProperty("schemaTableName") SchemaTableName schemaTableName,
40+
@JsonProperty("tablePath") String tablePath,
41+
@JsonProperty("storageType") StorageType storageType)
3042
{
3143
this.schemaTableName = schemaTableName;
44+
this.tablePath = tablePath;
45+
this.storageType = storageType;
3246
}
3347

3448
@JsonProperty
@@ -37,10 +51,22 @@ public SchemaTableName getSchemaTableName()
3751
return schemaTableName;
3852
}
3953

54+
@JsonProperty
55+
public String getTablePath()
56+
{
57+
return tablePath;
58+
}
59+
60+
@JsonProperty
61+
public StorageType getStorageType()
62+
{
63+
return storageType;
64+
}
65+
4066
@Override
4167
public int hashCode()
4268
{
43-
return Objects.hash(schemaTableName);
69+
return Objects.hash(schemaTableName, storageType);
4470
}
4571

4672
@Override
@@ -53,12 +79,16 @@ public boolean equals(Object obj)
5379
return false;
5480
}
5581
ClpTableHandle other = (ClpTableHandle) obj;
56-
return this.schemaTableName.equals(other.schemaTableName);
82+
return this.schemaTableName.equals(other.schemaTableName) &&
83+
this.storageType == other.storageType;
5784
}
5885

5986
@Override
6087
public String toString()
6188
{
62-
return schemaTableName.toString();
89+
return toStringHelper(this)
90+
.add("schemaTableName", schemaTableName)
91+
.add("storageType", storageType)
92+
.toString();
6393
}
6494
}

‎presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpTableLayoutHandle.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
import java.util.Objects;
2121
import java.util.Optional;
2222

23+
import static com.google.common.base.MoreObjects.toStringHelper;
24+
2325
public class ClpTableLayoutHandle
2426
implements ConnectorTableLayoutHandle
2527
{
@@ -28,7 +30,7 @@ public class ClpTableLayoutHandle
2830

2931
@JsonCreator
3032
public ClpTableLayoutHandle(@JsonProperty("table") ClpTableHandle table,
31-
@JsonProperty("query") Optional<String> kqlQuery)
33+
@JsonProperty("kqlQuery") Optional<String> kqlQuery)
3234
{
3335
this.table = table;
3436
this.kqlQuery = kqlQuery;
@@ -56,18 +58,22 @@ public boolean equals(Object o)
5658
return false;
5759
}
5860
ClpTableLayoutHandle that = (ClpTableLayoutHandle) o;
59-
return Objects.equals(table, that.table);
61+
return Objects.equals(table, that.table) &&
62+
Objects.equals(kqlQuery, that.kqlQuery);
6063
}
6164

6265
@Override
6366
public int hashCode()
6467
{
65-
return Objects.hash(table);
68+
return Objects.hash(table, kqlQuery);
6669
}
6770

6871
@Override
6972
public String toString()
7073
{
71-
return table.toString();
74+
return toStringHelper(this)
75+
.add("table", table)
76+
.add("kqlQuery", kqlQuery)
77+
.toString();
7278
}
7379
}

0 commit comments

Comments
 (0)