[ 
https://issues.apache.org/jira/browse/BEAM-6579?focusedWorklogId=194133&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194133
 ]

ASF GitHub Bot logged work on BEAM-6579:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 04/Feb/19 17:41
            Start Date: 04/Feb/19 17:41
    Worklog Time Spent: 10m 
      Work Description: reuvenlax commented on pull request #7714: [BEAM-6579] 
Fix create disposition for the case of temporary tables
URL: https://github.com/apache/beam/pull/7714#discussion_r253570074
 
 

 ##########
 File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/WriteTables.java
 ##########
 @@ -176,13 +176,23 @@ public void processElement(ProcessContext c, 
BoundedWindow window) throws Except
               c.sideInput(loadJobIdPrefixView), tableDestination, partition, 
c.pane().getIndex());
 
       if (!singlePartition) {
+        // This is a temp table. Create a new one for each partition and each 
pane.
         tableReference.setTableId(jobIdPrefix);
       }
 
-      WriteDisposition writeDisposition =
-          (c.pane().getIndex() == 0) ? firstPaneWriteDisposition : 
WriteDisposition.WRITE_APPEND;
-      CreateDisposition createDisposition =
-          (c.pane().getIndex() == 0) ? firstPaneCreateDisposition : 
CreateDisposition.CREATE_NEVER;
+      WriteDisposition writeDisposition = firstPaneWriteDisposition;
+      CreateDisposition createDisposition = firstPaneCreateDisposition;
+      if (c.pane().getIndex() > 0 && singlePartition) {
+        // If writing directly to the destination, then the table is created 
on the first write
+        // and we should change the disposition for subsequent writes.
+        writeDisposition = WriteDisposition.WRITE_APPEND;
+        createDisposition = CreateDisposition.CREATE_NEVER;
+      } else if (!singlePartition) {
+        // In this case, we are writing to a temp table and always need to 
create it.
 
 Review comment:
   I think this variable is poorly named - I'll rename it to noTempTables. It's 
true that a single partition implies there are no temp tables (because we can 
then load directly to the user's final table), but it's confusing in the code.
   
   Another point of confusion: these "partitions" are not BigQuery partitions, 
but rather file partitions that the connector creates when there are too many 
files for a single load job. To be fair, I think this code was written before 
BigQuery partitions existed.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 194133)
    Time Spent: 40m  (was: 0.5h)

> BigQueryIO  improperly handles triggering when temp tables are used
> -------------------------------------------------------------------
>
>                 Key: BEAM-6579
>                 URL: https://issues.apache.org/jira/browse/BEAM-6579
>             Project: Beam
>          Issue Type: Bug
>          Components: io-java-gcp
>    Affects Versions: 2.9.0
>            Reporter: Reuven Lax
>            Assignee: Reuven Lax
>            Priority: Major
>             Fix For: 2.10.0
>
>          Time Spent: 40m
>  Remaining Estimate: 0h
>
> In WriteTables.java, the create disposition is changed to CREATE_NEVER for 
> triggers after the first. This logic is correct when writing directly to the 
> destination table - the table is created on the first write, and does not 
> need to be recreated. However WriteTables is also used when writing to 
> temporary tables, and the logic is incorrect in that case as a new table is 
> created on every trigger.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to