Skip to content

Commit

Permalink
Remove support for Catalog -> ProjectId name mangling
Browse files Browse the repository at this point in the history
A prior version of this driver supported translating between SQL-92 and
BigQuery Legacy dialects.  As part of this support, it would mangle
BigQuery Project Ids like `looker.com:sample-project` to
`looker_com__sample-project`.  We found that there were some bugs in
this translation leading to the mangled name being sent to BigQuery when
the real ProjectId should have been sent.

We recently removed support for this translation between dialects.
Since we no longer require that, we can now remove most of the code
which did the name mangling.

In this PR, we fix on using the Project Id internally as much as
possible.  Only in public methods of the JDBC API do we accept a Catalog
name.  All internal methods translate this to a Project Id before doing
any further processing of them.

We do retain one vestige of the original name mangling.  We continue to
support the mangled names in the JDBC URI.  This is to maintain
compatibility with an existing configurations which used the mangled
name.  Should we decide to drop this support in the future, it should be
trivial now that all of the mangling is contained in the single
`CatalogName.toProjectId` method.
  • Loading branch information
kalenp committed Oct 27, 2020
1 parent 1a20d81 commit 38effb6
Show file tree
Hide file tree
Showing 9 changed files with 156 additions and 229 deletions.
35 changes: 16 additions & 19 deletions src/main/java/net/starschema/clouddb/jdbc/BQConnection.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,10 @@ public class BQConnection implements Connection {
private String dataset = null;

/**
* The projectid which needed for the queries.
* The ProjectId for the connection
*/
private String projectId = null;

/** Boolean to determine if the Connection is closed */
private boolean isclosed = false;

Expand Down Expand Up @@ -114,16 +115,14 @@ public BQConnection(String url, Properties loginProp) throws SQLException {
Matcher matchData = projectAndDatasetMatcher.matcher(pathParams);

if (matchData.find()) {
this.projectId = matchData.group(1);
this.projectId = CatalogName.toProjectId(matchData.group(1));
this.dataset = matchData.group(2);
} else {
this.projectId = pathParams;
this.projectId = CatalogName.toProjectId(pathParams);
}
} catch (UnsupportedEncodingException e1) {
throw new BQSQLException(e1);
}
// lets replace the : with __ and . with _
this.projectId = this.projectId.replace(":", "__").replace(".", "_");

Properties caseInsensitiveLoginProps = new Properties();

Expand Down Expand Up @@ -227,7 +226,7 @@ else if (oAuthAccessToken != null) {
else {
throw new IllegalArgumentException("Must provide a valid mechanism to authenticate.");
}
logger.debug("The project id for this connections is: " + this.projectId);
logger.debug("The project id for this connections is: " + projectId);
}

/**
Expand Down Expand Up @@ -371,7 +370,7 @@ public Statement createStatement() throws SQLException {
}
logger.debug("Creating statement with resultsettype: forward only," +
" concurrency: read only");
return new BQStatement(this.projectId, this);
return new BQStatement(projectId, this);
}

/** {@inheritDoc} */
Expand All @@ -383,8 +382,7 @@ public Statement createStatement(int resultSetType, int resultSetConcurrency)
}
logger.debug("Creating statement with resultsettype: " + resultSetType
+ " concurrency: " + resultSetConcurrency);
return new BQStatement(this.projectId, this, resultSetType,
resultSetConcurrency);
return new BQStatement(projectId, this, resultSetType, resultSetConcurrency);
}

/** {@inheritDoc} */
Expand Down Expand Up @@ -417,8 +415,8 @@ public void setSchema(String schema) {
}

@Override
public String getSchema() {
return this.dataset;
public String getSchema() throws SQLException {
return getDataSet();
}

public void abort(Executor executor) throws SQLException {
Expand Down Expand Up @@ -464,7 +462,7 @@ public Bigquery getBigquery() {
@Override
public String getCatalog() throws SQLException {
logger.debug("function call getCatalog returning projectId: " + projectId);
return this.projectId;
return projectId;
}

/**
Expand Down Expand Up @@ -526,7 +524,7 @@ public DatabaseMetaData getMetaData() throws SQLException {
* Getter method for projectId
*/
public String getProjectId() {
return this.projectId;
return projectId;
}

/**
Expand Down Expand Up @@ -638,7 +636,7 @@ public boolean isValid(int timeout) throws SQLException {
+ String.valueOf(timeout));
}
try {
this.bigquery.datasets().list(this.projectId.replace("__", ":").replace("_", ".")).execute();
this.bigquery.datasets().list(projectId).execute();
} catch (IOException e) {
return false;
}
Expand Down Expand Up @@ -729,10 +727,9 @@ public CallableStatement prepareCall(String sql, int resultSetType,
@Override
public PreparedStatement prepareStatement(String sql) throws SQLException {
this.logger.debug("Creating Prepared Statement project id is: "
+ this.projectId + " with parameters:");
+ projectId + " with parameters:");
this.logger.debug(sql);
PreparedStatement stm = new BQPreparedStatement(sql, this.projectId,
this);
PreparedStatement stm = new BQPreparedStatement(sql, projectId, this);
return stm;
}

Expand All @@ -756,12 +753,12 @@ public PreparedStatement prepareStatement(String sql, int autoGeneratedKeys)
public PreparedStatement prepareStatement(String sql, int resultSetType,
int resultSetConcurrency) throws SQLException {
this.logger.debug("Creating Prepared Statement" +
" project id is: " + this.projectId +
" project id is: " + projectId +
", resultSetType (int) is: " + String.valueOf(resultSetType) +
", resultSetConcurrency (int) is: " + String.valueOf(resultSetConcurrency)
+ " with parameters:");
this.logger.debug(sql);
PreparedStatement stm = new BQPreparedStatement(sql, this.projectId,
PreparedStatement stm = new BQPreparedStatement(sql, projectId,
this, resultSetType, resultSetConcurrency);
return stm;
}
Expand Down
65 changes: 34 additions & 31 deletions src/main/java/net/starschema/clouddb/jdbc/BQDatabaseMetadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ public ResultSet getCatalogs() throws SQLException {
String[] Data = new String[Projects.size()];
String toLog = "";
for (int i = 0; i < Projects.size(); i++) {
Data[i] = Projects.get(i).getId().replace(":", "__").replace(".", "_");
Data[i] = Projects.get(i).getId();
toLog += Data[i] + " , ";
}
logger.debug("Catalogs are: " + toLog);
Expand Down Expand Up @@ -402,11 +402,17 @@ public ResultSet getColumns(String catalog, String schemaPattern,
(columnNamePattern != null ? columnNamePattern : "null"));
List<Table> Tables = null;
try {
Tables = BQSupportFuncts.getTables(this.Connection, catalog,
schemaPattern, tableNamePattern);
Tables = BQSupportFuncts.getTables(
this.Connection,
CatalogName.toProjectId(catalog),
schemaPattern,
tableNamePattern);
if (Tables == null) { //Because of Crystal Reports It's not elegant, but hey it works!
Tables = BQSupportFuncts.getTables(this.Connection, schemaPattern,
catalog, tableNamePattern);
Tables = BQSupportFuncts.getTables(
this.Connection,
CatalogName.toProjectId(schemaPattern),
catalog,
tableNamePattern);
}
} catch (IOException e) {
throw new BQSQLException(e);
Expand All @@ -415,13 +421,9 @@ public ResultSet getColumns(String catalog, String schemaPattern,
if (Tables != null) {
List<String[]> data = new ArrayList<String[]>();
for (int i = 0; i < Tables.size(); i++) {
String UparsedId = Tables.get(i).getId();
String ProjectId = BQSupportFuncts
.getProjectIdFromAnyGetId(UparsedId).replace(":", "__").replace(".", "_");
String DatasetId = BQSupportFuncts
.getDatasetIdFromTableDotGetId(UparsedId);
String TableId = BQSupportFuncts
.getTableIdFromTableDotGetId(UparsedId);
String ProjectId = Tables.get(i).getTableReference().getProjectId();
String DatasetId = Tables.get(i).getTableReference().getDatasetId();
String TableId = Tables.get(i).getTableReference().getTableId();

List<TableFieldSchema> tblfldschemas = Tables.get(i)
.getSchema().getFields();
Expand Down Expand Up @@ -1404,10 +1406,8 @@ public ResultSet getSchemas() throws SQLException {
data = new String[datasetlist.size()][2];
int i = 0;
for (Datasets datasets : datasetlist) {
data[i][0] = datasets.getDatasetReference()
.getDatasetId();
data[i][1] = datasets.getDatasetReference()
.getProjectId().replace(".", "_").replace(":", "__");
data[i][0] = datasets.getDatasetReference().getDatasetId();
data[i][1] = datasets.getDatasetReference().getProjectId();
i++;
}
}
Expand Down Expand Up @@ -1482,10 +1482,8 @@ public ResultSet getSchemas(String catalog, String schemaPattern)
data = new String[datasetlist.size()][2];
i = 0;
for (Datasets datasets : datasetlist) {
String schema = datasets.getDatasetReference()
.getDatasetId();
String projnm = datasets.getDatasetReference()
.getProjectId();
String schema = datasets.getDatasetReference().getDatasetId();
String projnm = datasets.getDatasetReference().getProjectId();
logger.debug("We search for catalog/project: " + catalog);
if ((schema.equals(schemaPattern) || schemaPattern == null)
&& (projnm.equals(catalog) || catalog == null)) {
Expand Down Expand Up @@ -1744,11 +1742,17 @@ public ResultSet getTables(String catalog, String schemaPattern,
+ ", types: " + typesToLog + ")");
List<Table> tables = null;
try {
tables = BQSupportFuncts.getTables(this.Connection, catalog,
schemaPattern, tableNamePattern);
tables = BQSupportFuncts.getTables(
this.Connection,
CatalogName.toProjectId(catalog),
schemaPattern,
tableNamePattern);
if (tables == null) { //because of crystal reports, It's not elegant but hey, it works!
tables = BQSupportFuncts.getTables(this.Connection, tableNamePattern,
schemaPattern, catalog);
tables = BQSupportFuncts.getTables(
this.Connection,
CatalogName.toProjectId(tableNamePattern),
schemaPattern,
catalog);
}
} catch (IOException e) {
throw new BQSQLException(e);
Expand All @@ -1757,16 +1761,15 @@ public ResultSet getTables(String catalog, String schemaPattern,
logger.debug("got result, size: " + tables.size());
String[][] data = new String[tables.size()][10];
for (int i = 0; i < tables.size(); i++) {
String UparsedId = tables.get(i).getId();
data[i][0] = BQSupportFuncts.getProjectIdFromAnyGetId(UparsedId).replace(":", "__").replace(".", "_");
data[i][1] = BQSupportFuncts.getDatasetIdFromTableDotGetId(UparsedId);
data[i][2] = BQSupportFuncts.getTableIdFromTableDotGetId(UparsedId);
data[i][0] = tables.get(i).getTableReference().getProjectId();
data[i][1] = tables.get(i).getTableReference().getDatasetId();
data[i][2] = tables.get(i).getTableReference().getTableId();
data[i][3] = "TABLE";
data[i][4] = tables.get(i).getDescription();
data[i][5] = null;
data[i][6] = BQSupportFuncts.getProjectIdFromAnyGetId(UparsedId).replace(":", "__").replace(".", "_");
data[i][7] = BQSupportFuncts.getDatasetIdFromTableDotGetId(UparsedId);
data[i][8] = BQSupportFuncts.getTableIdFromTableDotGetId(UparsedId);
data[i][6] = tables.get(i).getTableReference().getProjectId();
data[i][7] = tables.get(i).getTableReference().getDatasetId();
data[i][8] = tables.get(i).getTableReference().getTableId();
data[i][9] = null;
}
return new DMDResultSet(data, new String[]{"TABLE_CAT",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public BQPreparedStatement(String querysql, String projectid,
this.logger.debug("Constructor of PreparedStatement Running " +
"projectid is:" + projectid + "sqlquery: " + querysql);

this.ProjectId = projectid;
this.projectId = projectid;
this.connection = bqConnection;
//this.resultSetType = ResultSet.TYPE_SCROLL_INSENSITIVE; //-scrollable
this.resultSetType = ResultSet.TYPE_FORWARD_ONLY;
Expand Down Expand Up @@ -116,7 +116,7 @@ public BQPreparedStatement(String querysql, String projectid,
"The Resultset Concurrency can't be ResultSet.CONCUR_UPDATABLE");
}

this.ProjectId = projectid;
this.projectId = projectid;
this.connection = bqConnection;
this.resultSetType = resultSetType;
this.resultSetConcurrency = resultSetConcurrency;
Expand Down Expand Up @@ -238,7 +238,7 @@ public ResultSet executeQuery() throws SQLException {
// Gets the Job reference of the completed job with give Query
referencedJob = BQSupportFuncts.startQuery(
this.connection.getBigquery(),
this.ProjectId.replace("__", ":").replace("_", "."),
this.projectId,
this.RunnableStatement,
this.connection.getDataSet(),
this.connection.getUseLegacySql(),
Expand All @@ -252,17 +252,17 @@ public ResultSet executeQuery() throws SQLException {
do {
if (BQSupportFuncts.getQueryState(referencedJob,
this.connection.getBigquery(),
this.ProjectId.replace("__", ":").replace("_", "."))
this.projectId)
.equals("DONE")) {
if (resultSetType == ResultSet.TYPE_SCROLL_INSENSITIVE) {
return new BQScrollableResultSet(BQSupportFuncts.getQueryResults(
this.connection.getBigquery(),
this.ProjectId.replace("__", ":").replace("_", "."),
this.projectId,
referencedJob), this);
} else {
return new BQForwardOnlyResultSet(
this.connection.getBigquery(),
this.ProjectId.replace("__", ":").replace("_", "."),
this.projectId,
referencedJob, this);
}
}
Expand Down
Loading

0 comments on commit 38effb6

Please sign in to comment.