Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor refactoring in Google Sheets connector #21757

Merged
merged 3 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.UncheckedExecutionException;
import com.google.inject.Inject;
import io.airlift.json.JsonCodec;
import io.airlift.log.Logger;
import io.trino.cache.EvictableCacheBuilder;
import io.trino.cache.NonEvictableLoadingCache;
Expand Down Expand Up @@ -85,10 +84,8 @@ public class SheetsClient
private final Sheets sheetsService;

@Inject
public SheetsClient(SheetsConfig config, JsonCodec<Map<String, List<SheetsTable>>> catalogCodec)
public SheetsClient(SheetsConfig config)
{
requireNonNull(catalogCodec, "catalogCodec is null");

this.metadataSheetId = config.getMetadataSheetId();

try {
Expand Down Expand Up @@ -144,19 +141,19 @@ public Optional<SheetsTable> getTableFromValues(List<List<Object>> values)
{
List<List<String>> stringValues = convertToStringValues(values);
if (stringValues.size() > 0) {
ImmutableList.Builder<SheetsColumn> columns = ImmutableList.builder();
ImmutableList.Builder<SheetsColumnHandle> columns = ImmutableList.builder();
Set<String> columnNames = new HashSet<>();
// Assuming 1st line is always header
List<String> header = stringValues.get(0);
int count = 0;
for (String column : header) {
String columnValue = column.toLowerCase(ENGLISH);
for (int i = 0; i < header.size(); i++) {
String columnValue = header.get(i).toLowerCase(ENGLISH);
// when empty or repeated column header, adding a placeholder column name
if (columnValue.isEmpty() || columnNames.contains(columnValue)) {
columnValue = "column_" + ++count;
}
columnNames.add(columnValue);
columns.add(new SheetsColumn(columnValue, VarcharType.VARCHAR));
columns.add(new SheetsColumnHandle(columnValue, VarcharType.VARCHAR, i));
}
List<List<String>> dataValues = stringValues.subList(1, values.size()); // removing header info
return Optional.of(new SheetsTable(columns.build(), dataValues));
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public record SheetsColumnHandle(
}

@JsonIgnore
public ColumnMetadata getColumnMetadata()
public ColumnMetadata columnMetadata()
{
return new ColumnMetadata(columnName, columnType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,14 @@
import io.trino.spi.function.table.ConnectorTableFunctionHandle;
import io.trino.spi.statistics.ComputedStatistics;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.Function;

import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.Iterables.getOnlyElement;
import static io.trino.plugin.google.sheets.SheetsConnectorTableHandle.tableNotFound;
import static io.trino.plugin.google.sheets.SheetsErrorCode.SHEETS_UNKNOWN_TABLE_ERROR;
Expand Down Expand Up @@ -96,7 +97,7 @@ public ConnectorTableMetadata getTableMetadata(ConnectorSession session, Connect
SheetsConnectorTableHandle tableHandle = (SheetsConnectorTableHandle) table;
SheetsTable sheetsTable = sheetsClient.getTable(tableHandle)
.orElseThrow(() -> new TrinoException(SHEETS_UNKNOWN_TABLE_ERROR, "Metadata not found for table " + tableNotFound(tableHandle)));
return new ConnectorTableMetadata(getSchemaTableName(tableHandle), sheetsTable.getColumnsMetadata());
return new ConnectorTableMetadata(getSchemaTableName(tableHandle), sheetsTable.columnsMetadata());
}

@Override
Expand All @@ -106,13 +107,8 @@ public Map<String, ColumnHandle> getColumnHandles(ConnectorSession session, Conn
SheetsTable table = sheetsClient.getTable(sheetsTableHandle)
.orElseThrow(() -> tableNotFound(sheetsTableHandle));

ImmutableMap.Builder<String, ColumnHandle> columnHandles = ImmutableMap.builder();
int index = 0;
for (ColumnMetadata column : table.getColumnsMetadata()) {
columnHandles.put(column.getName(), new SheetsColumnHandle(column.getName(), column.getType(), index));
index++;
}
return columnHandles.buildOrThrow();
return table.columns().stream()
.collect(toImmutableMap(SheetsColumnHandle::columnName, Function.identity()));
}

@Override
Expand All @@ -137,7 +133,7 @@ private Optional<ConnectorTableMetadata> getTableMetadata(SchemaTableName tableN
}
Optional<SheetsTable> table = sheetsClient.getTable(tableName.getTableName());
if (table.isPresent()) {
return Optional.of(new ConnectorTableMetadata(tableName, table.get().getColumnsMetadata()));
return Optional.of(new ConnectorTableMetadata(tableName, table.get().columnsMetadata()));
}
return Optional.empty();
}
Expand All @@ -164,7 +160,7 @@ public List<SchemaTableName> listTables(ConnectorSession session, Optional<Strin
@Override
public ColumnMetadata getColumnMetadata(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle columnHandle)
{
return ((SheetsColumnHandle) columnHandle).getColumnMetadata();
return ((SheetsColumnHandle) columnHandle).columnMetadata();
}

@Override
Expand All @@ -181,11 +177,7 @@ public ConnectorInsertTableHandle beginInsert(ConnectorSession session, Connecto
SheetsTable table = sheetsClient.getTable(namedTableHandle.tableName())
.orElseThrow(() -> new TableNotFoundException(namedTableHandle.getSchemaTableName()));

List<SheetsColumnHandle> columnHandles = new ArrayList<>(table.getColumnsMetadata().size());
for (int id = 0; id < table.getColumnsMetadata().size(); id++) {
columnHandles.add(new SheetsColumnHandle(table.getColumnsMetadata().get(id).getName(), table.getColumnsMetadata().get(id).getType(), id));
}
return new SheetsConnectorInsertTableHandle(namedTableHandle.tableName(), columnHandles);
return new SheetsConnectorInsertTableHandle(namedTableHandle.tableName(), table.columns());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@

import static com.google.inject.multibindings.Multibinder.newSetBinder;
import static io.airlift.configuration.ConfigBinder.configBinder;
import static io.airlift.json.JsonCodec.listJsonCodec;
import static io.airlift.json.JsonCodecBinder.jsonCodecBinder;

public class SheetsModule
implements Module
Expand All @@ -39,8 +37,6 @@ public void configure(Binder binder)

configBinder(binder).bindConfig(SheetsConfig.class);

jsonCodecBinder(binder).bindMapJsonCodec(String.class, listJsonCodec(SheetsTable.class));

newSetBinder(binder, ConnectorTableFunction.class).addBinding().toProvider(Sheet.class).in(Scopes.SINGLETON);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public ConnectorSplitSource getSplits(
.orElseThrow(() -> tableNotFound(tableHandle));

List<ConnectorSplit> splits = new ArrayList<>();
splits.add(new SheetsSplit(table.getValues()));
splits.add(new SheetsSplit(table.values()));
Collections.shuffle(splits);
return new FixedSplitSource(splits);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,43 +13,26 @@
*/
package io.trino.plugin.google.sheets;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.collect.ImmutableList;
import io.trino.spi.connector.ColumnMetadata;

import java.util.List;

import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.Objects.requireNonNull;

public class SheetsTable
public record SheetsTable(List<SheetsColumnHandle> columns, List<List<String>> values)
{
private final List<ColumnMetadata> columnsMetadata;
private final List<List<String>> values;

@JsonCreator
public SheetsTable(
@JsonProperty("columns") List<SheetsColumn> columns,
@JsonProperty("values") List<List<String>> values)
{
requireNonNull(columns, "columns is null");

ImmutableList.Builder<ColumnMetadata> columnsMetadata = ImmutableList.builder();
for (SheetsColumn column : columns) {
columnsMetadata.add(new ColumnMetadata(column.getName(), column.getType()));
}
this.columnsMetadata = columnsMetadata.build();
this.values = values;
}

@JsonProperty
public List<List<String>> getValues()
public SheetsTable
{
return values;
columns = ImmutableList.copyOf(requireNonNull(columns, "columns is null"));
values = ImmutableList.copyOf(requireNonNull(values, "values is null"));
}

public List<ColumnMetadata> getColumnsMetadata()
List<ColumnMetadata> columnsMetadata()
{
return columnsMetadata;
return columns.stream()
.map(SheetsColumnHandle::columnMetadata)
.collect(toImmutableList());
}
}