Skip to content

Commit bf9da2e

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
1 parent 550e448 commit bf9da2e

19 files changed

+204
-146
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: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,26 @@
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 StorageType storageType;
2736

2837
@JsonCreator
29-
public ClpTableHandle(@JsonProperty("schemaTableName") SchemaTableName schemaTableName)
38+
public ClpTableHandle(@JsonProperty("schemaTableName") SchemaTableName schemaTableName,
39+
@JsonProperty("storageType") StorageType storageType)
3040
{
3141
this.schemaTableName = schemaTableName;
42+
this.storageType = storageType;
3243
}
3344

3445
@JsonProperty
@@ -37,10 +48,16 @@ public SchemaTableName getSchemaTableName()
3748
return schemaTableName;
3849
}
3950

51+
@JsonProperty
52+
public StorageType getStorageType()
53+
{
54+
return storageType;
55+
}
56+
4057
@Override
4158
public int hashCode()
4259
{
43-
return Objects.hash(schemaTableName);
60+
return Objects.hash(schemaTableName, storageType);
4461
}
4562

4663
@Override
@@ -53,12 +70,16 @@ public boolean equals(Object obj)
5370
return false;
5471
}
5572
ClpTableHandle other = (ClpTableHandle) obj;
56-
return this.schemaTableName.equals(other.schemaTableName);
73+
return this.schemaTableName.equals(other.schemaTableName) &&
74+
this.storageType == other.storageType;
5775
}
5876

5977
@Override
6078
public String toString()
6179
{
62-
return schemaTableName.toString();
80+
return toStringHelper(this)
81+
.add("schemaTableName", schemaTableName)
82+
.add("storageType", storageType)
83+
.toString();
6384
}
6485
}

‎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
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
package com.facebook.presto.plugin.clp.metadata;
1515

1616
import com.facebook.presto.plugin.clp.ClpColumnHandle;
17+
import com.facebook.presto.plugin.clp.ClpTableHandle;
1718
import com.facebook.presto.spi.SchemaTableName;
1819

1920
import java.util.List;
@@ -23,10 +24,10 @@ public interface ClpMetadataProvider
2324
/**
2425
* Returns the list of column handles for the given table.
2526
*/
26-
public List<ClpColumnHandle> listColumnHandles(SchemaTableName schemaTableName);
27+
List<ClpColumnHandle> listColumnHandles(SchemaTableName schemaTableName);
2728

2829
/**
2930
* Returns the list of table names in the given schema.
3031
*/
31-
public List<String> listTableNames(String schema);
32+
List<ClpTableHandle> listTableHandles(String schema);
3233
}

0 commit comments

Comments
 (0)