ahmedabu98 commented on code in PR #28624:
URL: https://github.com/apache/beam/pull/28624#discussion_r1336153834


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableWriteSchemaTransformProvider.java:
##########
@@ -179,12 +179,13 @@ public KV<ByteString, Iterable<Mutation>> apply(Row row) {
                     .setColumnQualifier(
                         
ByteString.copyFrom(ofNullable(mutation.get("column_qualifier")).get()))
                     .setFamilyNameBytes(
-                        
ByteString.copyFrom(ofNullable(mutation.get("family_name")).get()));
-            if (mutation.containsKey("timestamp_micros")) {
-              setMutation =
-                  setMutation.setTimestampMicros(
-                      
Longs.fromByteArray(ofNullable(mutation.get("timestamp_micros")).get()));
-            }
+                        
ByteString.copyFrom(ofNullable(mutation.get("family_name")).get()))
+                    // Use timestamp if provided, else default to -1 (current 
Bigtable server time)
+                    .setTimestampMicros(

Review Comment:
   Sure I can agree with that. I can address this nit in another PR but for now 
the current changes should be okay to cherry-pick?
   
   @Abacn this means we'd have to make sure the Go wrapper defaults timestamps 
to time at ingestion. Also templates don't use the schematransform AFAIK so it 
shouldn't matter there anyways right? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to