Skip to content

Commit a824146

Browse files
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
1 parent 550e448 commit a824146

File tree

15 files changed

+104
-78
lines changed

15 files changed

+104
-78
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/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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,13 +155,13 @@ public Map<SchemaTableName, List<ColumnMetadata>> listTableColumns(ConnectorSess
155155
{
156156
requireNonNull(prefix, "prefix is null");
157157
String schemaName = prefix.getSchemaName();
158-
if (schemaName != null && !schemaName.equals(DEFAULT_SCHEMA_NAME)) {
158+
if (schemaName != null && !listSchemaNames(session).contains(schemaName)) {
159159
return ImmutableMap.of();
160160
}
161161

162162
List<SchemaTableName> schemaTableNames;
163163
if (prefix.getTableName() == null) {
164-
schemaTableNames = listTables(session, Optional.of(prefix.getSchemaName()));
164+
schemaTableNames = listTables(session, Optional.ofNullable(prefix.getSchemaName()));
165165
}
166166
else {
167167
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 splitPath)
3735
{
38-
this.splitPath = splitPath;
36+
this.path = splitPath;
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/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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ public interface ClpMetadataProvider
2323
/**
2424
* Returns the list of column handles for the given table.
2525
*/
26-
public List<ClpColumnHandle> listColumnHandles(SchemaTableName schemaTableName);
26+
List<ClpColumnHandle> listColumnHandles(SchemaTableName schemaTableName);
2727

2828
/**
2929
* Returns the list of table names in the given schema.
3030
*/
31-
public List<String> listTableNames(String schema);
31+
List<String> listTableNames(String schema);
3232
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public List<ClpColumnHandle> listColumnHandles(SchemaTableName schemaTableName)
8383
}
8484
}
8585
catch (SQLException e) {
86-
log.error("Failed to load table schema for %s: %s" + schemaTableName.getTableName(), e);
86+
log.warn("Failed to load table schema for %s: %s", schemaTableName.getTableName(), e);
8787
}
8888
return schemaTree.collectColumnHandles();
8989
}
@@ -108,7 +108,7 @@ public List<String> listTableNames(String schema)
108108
}
109109
}
110110
catch (SQLException e) {
111-
log.error("Failed to load table names: %s", e);
111+
log.warn("Failed to load table names: %s", e);
112112
}
113113
return tableNames;
114114
}

0 commit comments

Comments
 (0)