Skip to content

Conversation

@bhshkh
Copy link
Contributor

@bhshkh bhshkh commented Mar 28, 2025

if complete batch data is abcd and it's split into 2 chunks, result sets should be

partialResultSet {
    protoRowsBatch{ 
        batchData: 'ab'
    }
    reset = true
}


partialResultSet {   
    protoRowsBatch{       
        batchData: 'cd'   
    }   
    reset = false
    batch_checksum = crc32c(abcd)
} 

Currently, newExecQueryRespPartialBatchFirstHalf returns a resp with batchCheckSum. This means it is actually a complete batch and not just first half of the batch. i.e

partialResultSet {
    protoRowsBatch{ 
        batchData: 'ab'
    }
    reset = true
    batch_checksum = crc32c(ab)
}


partialResultSet {   
    protoRowsBatch{       
        batchData: 'cd'   
    }   
    reset = false
    batch_checksum = crc32c(cd)
} 

Updating the test to fix this

@product-auto-label product-auto-label bot added the api: bigtable Issues related to the Bigtable API. label Mar 28, 2025
@bhshkh bhshkh marked this pull request as ready for review March 28, 2025 00:08
@bhshkh bhshkh requested review from a team as code owners March 28, 2025 00:08
@bhshkh bhshkh requested review from a team, gkevinzheng and jackdingilian March 28, 2025 00:08
Copy link

@jackdingilian jackdingilian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test case that tests partial batch reset?

  • newExecQueryRespPartialBatchFirstHalf(reset=true, data="foo")
  • unavailable
  • newExecQueryRespPartialBatchFirstHalf(reset=true, data="bar")
  • newExecQueryRespPartialBatchSecondHalf(reset=false, data="baz")
  • EOF

And we should only have bar,baz as the results

@bhshkh
Copy link
Contributor Author

bhshkh commented Mar 29, 2025

Can you add a test case that tests partial batch reset?

  • newExecQueryRespPartialBatchFirstHalf(reset=true, data="foo")
  • unavailable
  • newExecQueryRespPartialBatchFirstHalf(reset=true, data="bar")
  • newExecQueryRespPartialBatchSecondHalf(reset=false, data="baz")
  • EOF

And we should only have bar,baz as the results

Added

@bhshkh bhshkh enabled auto-merge (squash) March 31, 2025 15:54
@bhshkh bhshkh merged commit d593902 into googleapis:main Mar 31, 2025
10 checks passed
@bhshkh bhshkh linked an issue Oct 8, 2025 that may be closed by this pull request
@bhshkh bhshkh deleted the feature/cbt-btql-execute-03 branch October 17, 2025 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the Bigtable API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bigtable: GoogleSQL test fixes

3 participants