Skip to content

Commit 50fded9

Browse files
wraymowraymo
committed
apply more fixes
1 parent 2f0424d commit 50fded9

File tree

10 files changed

+41
-57
lines changed

10 files changed

+41
-57
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,9 @@ public String getMetadataTablePrefix()
114114
public ClpConfig setMetadataTablePrefix(String metadataTablePrefix)
115115
{
116116
if (metadataTablePrefix == null || !SAFE_SQL_TABLE_NAME_PATTERN.matcher(metadataTablePrefix).matches()) {
117-
throw new PrestoException(ClpErrorCode.CLP_UNSUPPORTED_CONFIG_OPTION, "Invalid metadataTablePrefix: " +
118-
metadataTablePrefix + ". Only alphanumeric characters and underscores are allowed.");
117+
throw new PrestoException(
118+
ClpErrorCode.CLP_UNSUPPORTED_CONFIG_OPTION,
119+
"Invalid metadataTablePrefix: " + metadataTablePrefix + ". Only alphanumeric characters and underscores are allowed.");
119120
}
120121

121122
this.metadataTablePrefix = metadataTablePrefix;

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,7 @@ public class ClpConnector
3737
private final ClpSplitManager splitManager;
3838

3939
@Inject
40-
public ClpConnector(
41-
LifeCycleManager lifeCycleManager,
42-
ClpMetadata metadata,
43-
ClpRecordSetProvider recordSetProvider,
44-
ClpSplitManager splitManager)
40+
public ClpConnector(LifeCycleManager lifeCycleManager, ClpMetadata metadata, ClpRecordSetProvider recordSetProvider, ClpSplitManager splitManager)
4541
{
4642
this.lifeCycleManager = requireNonNull(lifeCycleManager, "lifeCycleManager is null");
4743
this.metadata = requireNonNull(metadata, "metadata is null");

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,14 @@ protected void setup(Binder binder)
4141
binder.bind(ClpMetadataProvider.class).to(ClpMySqlMetadataProvider.class).in(Scopes.SINGLETON);
4242
}
4343
else {
44-
throw new PrestoException(ClpErrorCode.CLP_UNSUPPORTED_METADATA_SOURCE,
45-
"Unsupported metadata provider type: " + config.getMetadataProviderType());
44+
throw new PrestoException(ClpErrorCode.CLP_UNSUPPORTED_METADATA_SOURCE, "Unsupported metadata provider type: " + config.getMetadataProviderType());
4645
}
4746

4847
if (config.getSplitProviderType() == ClpConfig.SplitProviderType.MYSQL) {
4948
binder.bind(ClpSplitProvider.class).to(ClpMySqlSplitProvider.class).in(Scopes.SINGLETON);
5049
}
5150
else {
52-
throw new PrestoException(ClpErrorCode.CLP_UNSUPPORTED_SPLIT_SOURCE,
53-
"Unsupported split provider type: " + config.getSplitProviderType());
51+
throw new PrestoException(ClpErrorCode.CLP_UNSUPPORTED_SPLIT_SOURCE, "Unsupported split provider type: " + config.getSplitProviderType());
5452
}
5553
}
5654
}

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,7 @@ public class ClpRecordSetProvider
2626
implements ConnectorRecordSetProvider
2727
{
2828
@Override
29-
public RecordSet getRecordSet(
30-
ConnectorTransactionHandle transactionHandle,
31-
ConnectorSession session,
32-
ConnectorSplit split,
33-
List<? extends ColumnHandle> columns)
29+
public RecordSet getRecordSet(ConnectorTransactionHandle transactionHandle, ConnectorSession session, ConnectorSplit split, List<? extends ColumnHandle> columns)
3430
{
3531
throw new UnsupportedOperationException("getRecordSet is not supported");
3632
}

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

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,13 @@ public class ClpMySqlMetadataProvider
4747
public static final String DATASETS_TABLE_SUFFIX = "datasets";
4848

4949
// SQL templates
50-
private static final String SQL_SELECT_COLUMN_METADATA_TEMPLATE = "SELECT * FROM `%s%s" +
51-
COLUMN_METADATA_TABLE_SUFFIX + "`";
52-
private static final String SQL_SELECT_DATASETS_TEMPLATE =
53-
format("SELECT `%s`, `%s`, `%s` FROM `%%s%s`", DATASETS_TABLE_COLUMN_NAME,
54-
DATASETS_TABLE_COLUMN_ARCHIVE_STORAGE_TYPE, DATASETS_TABLE_COLUMN_ARCHIVE_STORAGE_DIRECTORY,
55-
DATASETS_TABLE_SUFFIX);
50+
private static final String SQL_SELECT_COLUMN_METADATA_TEMPLATE = "SELECT * FROM `%s%s" + COLUMN_METADATA_TABLE_SUFFIX + "`";
51+
private static final String SQL_SELECT_DATASETS_TEMPLATE = format(
52+
"SELECT `%s`, `%s`, `%s` FROM `%%s%s`",
53+
DATASETS_TABLE_COLUMN_NAME,
54+
DATASETS_TABLE_COLUMN_ARCHIVE_STORAGE_TYPE,
55+
DATASETS_TABLE_COLUMN_ARCHIVE_STORAGE_DIRECTORY,
56+
DATASETS_TABLE_SUFFIX);
5657

5758
private static final Logger log = Logger.get(ClpMySqlMetadataProvider.class);
5859

@@ -74,14 +75,13 @@ public ClpMySqlMetadataProvider(ClpConfig config)
7475
@Override
7576
public List<ClpColumnHandle> listColumnHandles(SchemaTableName schemaTableName)
7677
{
77-
String query = format(SQL_SELECT_COLUMN_METADATA_TEMPLATE,
78-
config.getMetadataTablePrefix(), schemaTableName.getTableName());
78+
String query = format(SQL_SELECT_COLUMN_METADATA_TEMPLATE, config.getMetadataTablePrefix(), schemaTableName.getTableName());
7979
ClpSchemaTree schemaTree = new ClpSchemaTree(config.isPolymorphicTypeEnabled());
80-
try (Connection connection = getConnection();
81-
PreparedStatement statement = connection.prepareStatement(query)) {
80+
try (Connection connection = getConnection(); PreparedStatement statement = connection.prepareStatement(query)) {
8281
try (ResultSet resultSet = statement.executeQuery()) {
8382
while (resultSet.next()) {
84-
schemaTree.addColumn(resultSet.getString(COLUMN_METADATA_TABLE_COLUMN_NAME),
83+
schemaTree.addColumn(
84+
resultSet.getString(COLUMN_METADATA_TABLE_COLUMN_NAME),
8585
resultSet.getByte(COLUMN_METADATA_TABLE_COLUMN_TYPE));
8686
}
8787
}
@@ -104,11 +104,12 @@ public List<ClpTableHandle> listTableHandles(String schemaName)
104104
String tableName = resultSet.getString(DATASETS_TABLE_COLUMN_NAME);
105105
String archiveStorageType = resultSet.getString(DATASETS_TABLE_COLUMN_ARCHIVE_STORAGE_TYPE);
106106
String archiveStorageDirectory = resultSet.getString(DATASETS_TABLE_COLUMN_ARCHIVE_STORAGE_DIRECTORY);
107-
if (isValidIdentifier(tableName) && archiveStorageDirectory != null &&
108-
!archiveStorageDirectory.isEmpty()) {
109-
tableHandles.add(new ClpTableHandle(new SchemaTableName(schemaName, tableName),
110-
archiveStorageDirectory,
111-
ClpTableHandle.StorageType.valueOf(archiveStorageType.toUpperCase())));
107+
if (isValidIdentifier(tableName) && archiveStorageDirectory != null && !archiveStorageDirectory.isEmpty()) {
108+
tableHandles.add(
109+
new ClpTableHandle(
110+
new SchemaTableName(schemaName, tableName),
111+
archiveStorageDirectory,
112+
ClpTableHandle.StorageType.valueOf(archiveStorageType.toUpperCase())));
112113
}
113114
else {
114115
log.warn("Ignoring invalid table name found in metadata: %s", tableName);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ private Type buildRowType(ClpNode node)
182182
return RowType.from(fields);
183183
}
184184

185-
static class ClpNode
185+
static private class ClpNode
186186
{
187187
Type type; // Only non-null for leaf nodes
188188
String originalName;

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,10 @@ public class ClpMySqlSplitProvider
3838
public static final String ARCHIVES_TABLE_COLUMN_ID = "id";
3939

4040
// Table suffixes
41-
private static final String ARCHIVE_TABLE_SUFFIX = "_archives";
41+
public static final String ARCHIVE_TABLE_SUFFIX = "_archives";
4242

4343
// SQL templates
44-
private static final String SQL_SELECT_ARCHIVES_TEMPLATE =
45-
format("SELECT `%s` FROM `%%s%%s%s`", ARCHIVES_TABLE_COLUMN_ID, ARCHIVE_TABLE_SUFFIX);
44+
private static final String SQL_SELECT_ARCHIVES_TEMPLATE = format("SELECT `%s` FROM `%%s%%s%s`", ARCHIVES_TABLE_COLUMN_ID, ARCHIVE_TABLE_SUFFIX);
4645

4746
private static final Logger log = Logger.get(ClpMySqlSplitProvider.class);
4847

@@ -72,8 +71,7 @@ public List<ClpSplit> listSplits(ClpTableLayoutHandle clpTableLayoutHandle)
7271

7372
try (Connection connection = getConnection()) {
7473
// Fetch archive IDs and create splits
75-
try (PreparedStatement statement = connection.prepareStatement(archivePathQuery);
76-
ResultSet resultSet = statement.executeQuery()) {
74+
try (PreparedStatement statement = connection.prepareStatement(archivePathQuery); ResultSet resultSet = statement.executeQuery()) {
7775
while (resultSet.next()) {
7876
final String archiveId = resultSet.getString(ARCHIVES_TABLE_COLUMN_ID);
7977
final String archivePath = tablePath + "/" + archiveId;

‎presto-clp/src/test/java/com/facebook/presto/plugin/clp/ClpMetadataDbSetUp.java‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,15 @@
3939
import static com.facebook.presto.plugin.clp.metadata.ClpMySqlMetadataProvider.DATASETS_TABLE_COLUMN_NAME;
4040
import static com.facebook.presto.plugin.clp.metadata.ClpMySqlMetadataProvider.DATASETS_TABLE_SUFFIX;
4141
import static com.facebook.presto.plugin.clp.split.ClpMySqlSplitProvider.ARCHIVES_TABLE_COLUMN_ID;
42+
import static com.facebook.presto.plugin.clp.split.ClpMySqlSplitProvider.ARCHIVE_TABLE_SUFFIX;
4243
import static java.lang.String.format;
4344
import static org.testng.Assert.fail;
4445

4546
public final class ClpMetadataDbSetUp
4647
{
4748
public static final String METADATA_DB_PASSWORD = "";
4849
public static final String METADATA_DB_TABLE_PREFIX = "clp_";
49-
public static final String METADATA_DB_URL_TEMPLATE =
50-
"jdbc:h2:file:%s;MODE=MySQL;DATABASE_TO_UPPER=FALSE";
50+
public static final String METADATA_DB_URL_TEMPLATE = "jdbc:h2:file:%s;MODE=MySQL;DATABASE_TO_UPPER=FALSE";
5151
public static final String METADATA_DB_USER = "sa";
5252
public static final String ARCHIVE_STORAGE_DIRECTORY_BASE = "/tmp/archives/";
5353

@@ -70,8 +70,7 @@ public static ClpMetadata setupMetadata(DbHandle dbHandle, Map<String, List<Pair
7070
final String metadataDbUrl = format(METADATA_DB_URL_TEMPLATE, dbHandle.dbPath);
7171
final String columnMetadataTableSuffix = "_column_metadata";
7272

73-
try (Connection conn = DriverManager.getConnection(metadataDbUrl, METADATA_DB_USER, METADATA_DB_PASSWORD);
74-
Statement stmt = conn.createStatement()) {
73+
try (Connection conn = DriverManager.getConnection(metadataDbUrl, METADATA_DB_USER, METADATA_DB_PASSWORD); Statement stmt = conn.createStatement()) {
7574
createDatasetsTable(stmt);
7675

7776
for (Map.Entry<String, List<Pair<String, ClpSchemaTreeNodeType>>> entry : clpFields.entrySet()) {
@@ -110,7 +109,8 @@ public static ClpMetadata setupMetadata(DbHandle dbHandle, Map<String, List<Pair
110109
fail(e.getMessage());
111110
}
112111

113-
ClpConfig config = new ClpConfig().setPolymorphicTypeEnabled(true)
112+
ClpConfig config = new ClpConfig()
113+
.setPolymorphicTypeEnabled(true)
114114
.setMetadataDbUrl(metadataDbUrl)
115115
.setMetadataDbUser(METADATA_DB_USER)
116116
.setMetadataDbPassword(METADATA_DB_PASSWORD)
@@ -122,11 +122,9 @@ public static ClpMetadata setupMetadata(DbHandle dbHandle, Map<String, List<Pair
122122
public static ClpMySqlSplitProvider setupSplit(DbHandle dbHandle, Map<String, List<String>> splits)
123123
{
124124
final String metadataDbUrl = format(METADATA_DB_URL_TEMPLATE, dbHandle.dbPath);
125-
final String archiveTableSuffix = "_archives";
126-
final String archiveTableFormat = METADATA_DB_TABLE_PREFIX + "%s" + archiveTableSuffix;
125+
final String archiveTableFormat = METADATA_DB_TABLE_PREFIX + "%s" + ARCHIVE_TABLE_SUFFIX;
127126

128-
try (Connection conn = DriverManager.getConnection(metadataDbUrl, METADATA_DB_USER, METADATA_DB_PASSWORD);
129-
Statement stmt = conn.createStatement()) {
127+
try (Connection conn = DriverManager.getConnection(metadataDbUrl, METADATA_DB_USER, METADATA_DB_PASSWORD); Statement stmt = conn.createStatement()) {
130128
createDatasetsTable(stmt);
131129

132130
// Create and populate archive tables
@@ -145,10 +143,7 @@ public static ClpMySqlSplitProvider setupSplit(DbHandle dbHandle, Map<String, Li
145143

146144
stmt.execute(createArchiveTableSQL);
147145

148-
String insertArchiveTableSQL = format(
149-
"INSERT INTO %s (%s) VALUES (?)",
150-
archiveTableName,
151-
ARCHIVES_TABLE_COLUMN_ID);
146+
String insertArchiveTableSQL = format("INSERT INTO %s (%s) VALUES (?)", archiveTableName, ARCHIVES_TABLE_COLUMN_ID);
152147
try (PreparedStatement pstmt = conn.prepareStatement(insertArchiveTableSQL)) {
153148
for (String splitPath : tableSplits.getValue()) {
154149
pstmt.setString(1, splitPath);

‎presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpMetadata.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
@Test(singleThreaded = true)
4242
public class TestClpMetadata
4343
{
44-
private static final String tableName = "test";
44+
private static final String TABLE_NAME = "test";
4545
private ClpMetadataDbSetUp.DbHandle dbHandle;
4646
private ClpMetadata metadata;
4747

@@ -52,7 +52,7 @@ public void setUp()
5252
metadata = ClpMetadataDbSetUp.setupMetadata(
5353
dbHandle,
5454
ImmutableMap.of(
55-
tableName,
55+
TABLE_NAME,
5656
ImmutableList.of(
5757
new Pair<>("a", ClpSchemaTreeNodeType.Integer),
5858
new Pair<>("a", ClpSchemaTreeNodeType.VarString),
@@ -79,15 +79,14 @@ public void testListSchemaNames()
7979
public void testListTables()
8080
{
8181
HashSet<SchemaTableName> tables = new HashSet<>();
82-
tables.add(new SchemaTableName(DEFAULT_SCHEMA_NAME, tableName));
82+
tables.add(new SchemaTableName(DEFAULT_SCHEMA_NAME, TABLE_NAME));
8383
assertEquals(new HashSet<>(metadata.listTables(SESSION, Optional.empty())), tables);
8484
}
8585

8686
@Test
8787
public void testGetTableMetadata()
8888
{
89-
ClpTableHandle clpTableHandle =
90-
(ClpTableHandle) metadata.getTableHandle(SESSION, new SchemaTableName(DEFAULT_SCHEMA_NAME, tableName));
89+
ClpTableHandle clpTableHandle = (ClpTableHandle) metadata.getTableHandle(SESSION, new SchemaTableName(DEFAULT_SCHEMA_NAME, TABLE_NAME));
9190
ConnectorTableMetadata tableMetadata = metadata.getTableMetadata(SESSION, clpTableHandle);
9291
ImmutableSet<ColumnMetadata> columnMetadata = ImmutableSet.<ColumnMetadata>builder()
9392
.add(ColumnMetadata.builder()

‎presto-docs/src/main/sphinx/connector/clp.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ The CLP connector relies on metadata and split providers to retrieve information
9090
uses a MySQL database for both metadata and split storage. We recommend using the CLP package for log ingestion, which
9191
automatically populates the database with the required information.
9292

93-
If you prefer to use a different source—-or the same source with a custom implementation—you can provide your own
93+
If you prefer to use a different source--or the same source with a custom implementation—you can provide your own
9494
implementations of the ``ClpMetadataProvider`` and ``ClpSplitProvider`` interfaces, and configure the connector
9595
accordingly.
9696

0 commit comments

Comments
 (0)