Change in TableId behaviour for non default BigQueryClient project 1.44 -> 1.45

[NickApostolidesMatillion]

When using the getTable method of the BigQuery client the project of the TableId is overwritten. TableId would previously ignore this if a project had been set. Table Id has been changed such that the project is always overwritten. There is now no way to get a table for a project other than the one the client was created for.

TableId tableId = TableId.of(project, schema, tableName);
Table table = client.getTable(tableId);
// table returns null if project != client.getOptions().getProjectId()

this is due to the change in TableId

In BigQueryImpl:

public Table getTable(TableId tableId, TableOption... options) {
    final TableId completeTableId = tableId.setProjectId(getOptions().getProjectId());
    ...

TableId old code:

TableId setProjectId(String projectId) {
  return getProject() != null ? this : TableId.of(projectId, getDataset(), getTable());
}

TableId new code:

TableId setProjectId(String projectId) {
  Preconditions
    .checkArgument(!Strings.isNullOrEmpty(projectId), "Provided projectId is null or empty");
  return TableId.of(projectId, getDataset(), getTable());
}

[MMMarcy]

@NickApostolidesMatillion I'm the author of that change. It was due to #3283 and this behavior is definitely unintended. However, reverting the change will re-open the mentioned issue.

Perhaps something like:

public Table getTable(TableId tableId, TableOption... options) {

    final TableId completeTableId = tableId.setProjectId(
        tableId.getProjectId() == null
        ? getOptions().getProjectId()
        : tableId.getProjectId()
    );
...
}

Would solve your issue and still keep the fix for #3283 (granted that now you supply a TableId with the desired projectId already set).

[NickApostolidesMatillion]

@MMMarcy that would work for us. It is more suitable to have this logic in BigQueryImpl.

Thanks

[MMMarcy]

Perfect. Waiting for the owners thoughts now. If positive, I can open a PR

[MMMarcy]

Actually, thinking more about this using the proposed solution I mentioned hides too much of the behavior to the caller. Maybe overloading the getTable method with

public Table getTable(TableId tableId, ProjectId projectId, TableOption... options) {
...
}

seems a better alternative.

[NickApostolidesMatillion]

TableId already has a factory method including the project:

public static TableId of(String project, String dataset, String table)

I would suggest that if the project is set in TableId, the developer knows the project he/she wants.

[polleyg]

Unless I've missed something obvious folks, I think this issue needs to be reopened. The PR only fixed the getTable() method, but the same bug exists for other methods like delete(TableId tableId) and update(TableInfo tableInfo). Other methods like create() and update() also appear to have the same issue.

[NickApostolidesMatillion]

@MMMarcy Can this issue be reopened and all occurrences be fixed or the original change reverted?

[chingor13]

@JesseLovelace PTAL

[polleyg]

Hey Folks - happy to open new issue(s) if that's what is needed to get this fixed up? Just let me know.

[polleyg]

Hey Folks - any chance of getting someone to look at this? It's becoming a bit of a problem now. Maybe reverting the original change should be considered.

[mikekap]

I think there's way more callsites of setProjectId that aren't fixed - including insertAll that are still broken. IntelliJ shows 15 usages of which 3 are fixed now.

[mikekap]

@MMMarcy given the extent of the breakage - what was the motivation behind the original change? The setProjectId method is package-private so it shouldn't have been called externally.

[sduskis]

@pmakani, it looks like there are more places that need to change.

[chingor13]

@pmakani Did #4196 actually fix all the cases?

[polleyg]

@chingor13 I was about to ask the same. He's all the references I see to setProject() in my IDEA..

[lepistone]

Hi, we believe this is still broken as of today (1.65.0). As in the description of this ticket, still There is now no way to get a table for a project other than the one the client was created for.

Specifically, in https://github.com/googleapis/google-cloud-java/blob/master/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java#L635-L639 , the project in my tableId will always be overridden with the project of the service, if the service has a project.

Maybe the intention of that code was to override the project only if it is not set in tableId? Then the code would look like

          tableId.setProjectId(
              Strings.isNullOrEmpty(tableId.getProjectId())
                  ? serviceOptions.getProjectId());
                  : tableId.getProject()

Or maybe, the confusion arose between TableId and TableInfo. The code above calls TableId::setProject, which will always perform the change. See: https://github.com/googleapis/google-cloud-java/blob/master/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/TableId.java#L100

Thanks!