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

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());
}

@NickApostolidesMatillion

@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).

@MMMarcy

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

Thanks

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

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.

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.

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.

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.

@MMMarcy

@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.

@pmakani

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

@pmakani

@pmakani Did #4196 actually fix all the cases?

@chingor13

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

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!

Fixes googleapis/google-cloud-java#3808. (#3873)

Navigation Menu

Repository navigation

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

12 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Activity

MMMarcy commented on Oct 17, 2018

NickApostolidesMatillion commented on Oct 17, 2018

MMMarcy commented on Oct 17, 2018

MMMarcy commented on Oct 17, 2018

NickApostolidesMatillion commented on Oct 17, 2018

12 remaining items

polleyg commented on Nov 22, 2018

mikekap commented on Dec 3, 2018

mikekap commented on Dec 6, 2018

sduskis commented on Dec 6, 2018

chingor13 commented on Dec 12, 2018

polleyg commented on Dec 13, 2018

lepistone commented on Mar 21, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions