Implementing DataSource V2 Read Path by wchevreuil · Pull Request #162 · apache/hbase-connectors

wchevreuil · 2026-08-28T13:51:39Z

No description provided.

Copilot

Pull request overview

Implements the Spark 4 DataSource V2 read path for HBase, including filter pushdown, partition planning, and row conversion.

Changes:

Adds the V2 provider, table, scan, batch, partition, and reader pipeline.
Ports row-key range and predicate handling.
Adds unit and mini-cluster integration tests.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`ScanRange.scala`	Adds row-key range operations.
`HBaseTableProvider.scala`	Adds the V2 provider entry point.
`HBaseTable.scala`	Declares table schema and read capability.
`HBaseScanBuilder.scala`	Negotiates filters and projections.
`HBaseScan.scala`	Builds the logical HBase scan.
`HBaseBatch.scala`	Plans region-based input partitions.
`HBaseInputPartition.scala`	Defines serializable partitions.
`HBasePartitionReaderFactory.scala`	Creates partition readers.
`HBasePartitionReader.scala`	Executes scans and creates rows.
`ScanRangeSuite.scala`	Tests range operations.
`HBaseScanBuilderSuite.scala`	Tests filter and column pushdown.
`HBaseTableCatalogSuite.scala`	Tests catalog parsing.
`HBaseTableProviderSuite.scala`	Tests end-to-end reads.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

taklwu

may you fix the commit title with JIRA prefix HBASE-30189 ?

taklwu · 2026-09-01T03:24:43Z

+                .orElse(intersectedPoints.lastOption.map(Utils.incrementByteArray))
+                .orElse(region.end)
+                .orNull
+              Some(HBaseInputPartition(region.index, startRow, stopRow): InputPartition)


seems like a behavior difference from V1 that HBaseInputPartition that uses HBasePartitionReader.scala are scanning instead of Get , do you think this is good ? if not , please try to align what Spark3 does with Get.

We should use Get too. Changing this in the next commit.

taklwu · 2026-09-01T03:28:34Z

+    extends Batch
+    with Logging {
+
+  override def planInputPartitions(): Array[InputPartition] = {


we may need to support getPreferredLocations like V1 does , you can have it in the future PR.

This method is defined in the RDD class, which is the model used in V1. V2 uses the Batch model, so there's no RDD subclassing.

wchevreuil · 2026-09-08T16:15:29Z

may you fix the commit title with JIRA prefix HBASE-30189 ?

I'm actually targeting this PR to the HBASE-30189 branch, which already has two previous commits related to this HBASE-30189. The idea was to split HBASE-30189 into several PRs for ease of review. After all PRs are in HBASE-30189, I'm going to merge it into master as a single commit for HBASE-30189.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.

Apache-HBase · 2026-09-09T16:09:37Z

(!) A patch to the testing environment has been detected.
Re-executing against the patched versions to perform further tests.
The console is at https://ci-hbase.apache.org/job/HBase-Connectors-PreCommit/job/PR-162/7/console in case of problems.

Apache-HBase · 2026-09-09T16:13:57Z

(!) A patch to the testing environment has been detected.
Re-executing against the patched versions to perform further tests.
The console is at https://ci-hbase.apache.org/job/HBase-Connectors-PreCommit/job/PR-162/8/console in case of problems.

…job (apache#164) Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org> Signed-off-by: Dávid Paksy <paksyd@apache.org> Reviewed-by: Kevin Geiszler <kevin.j.geiszler@gmail.com>

Co-authored-by: Claude Code (claude-opus-4-6) <no-reply@anthropic.com> Signed-off-by: Peter Somogyi <psomogyi@apache.org>

…emas (apache#160) Co-authored-by: Claude Code (claude-opus-4-6) <no-reply@anthropic.com> Signed-off-by: Peter Somogyi <psomogyi@apache.org>

Apache-HBase · 2026-09-10T09:59:02Z

(!) A patch to the testing environment has been detected.
Re-executing against the patched versions to perform further tests.
The console is at https://ci-hbase.apache.org/job/HBase-Connectors-PreCommit/job/PR-162/10/console in case of problems.

Copilot

🟡 Changes recommended

Composite keys, unions, null handling, and configuration propagation still contain correctness or scalability defects.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

spark4/hbase-spark4/src/main/scala/org/apache/hadoop/hbase/spark/datasources/HBasePartitionReader.scala:198

The same missing-qualifier problem affects point reads: a Get restricted to the IsNull qualifier returns an empty Result when that qualifier is absent, and line 202 drops it before the predicate can accept it. For pushed expressions that can match a missing cell, do not restrict the Get to only those qualifiers (or otherwise fetch an existence column).

        scanFields.foreach { f =>
          g.addColumn(f.cfBytes, f.colBytes)
        }

Files reviewed: 17/17 changed files
Comments generated: 8
Review effort level: Balanced

+    val conf = HBaseConfiguration.create()
+    val configResources = properties.get(HBaseSparkConf.HBASE_CONFIG_LOCATION)
+    configResources.foreach(_.split(",").foreach(r => conf.addResource(new Path(r))))


+    scanFields.foreach { f =>
+      scan.addColumn(f.cfBytes, f.colBytes)
+    }


wchevreuil · 2026-09-10T14:55:40Z

+      case EqualTo(attr, _) => catalog.sMap.map.contains(attr)
+      case LessThan(attr, _) => catalog.sMap.map.contains(attr)
+      case GreaterThan(attr, _) => catalog.sMap.map.contains(attr)
+      case LessThanOrEqual(attr, _) => catalog.sMap.map.contains(attr)
+      case GreaterThanOrEqual(attr, _) => catalog.sMap.map.contains(attr)
+      case StringStartsWith(attr, _) => catalog.sMap.map.contains(attr)
+      case IsNull(attr) => catalog.sMap.map.contains(attr)
+      case IsNotNull(attr) => catalog.sMap.map.contains(attr)


Addressing on next comment.

wchevreuil · 2026-09-10T14:46:30Z

+  def mergeUnion(other: RowKeyFilter): RowKeyFilter = {
+    other.points.foreach(p => points += p)
+
+    other.ranges.foreach { otherR =>
+      var doesOverLap = false
+      ranges.foreach { r =>
+        if (r.getOverLapScanRange(otherR) != null) {
+          r.mergeUnion(otherR)
+          doesOverLap = true
+        }
+      }
+      if (!doesOverLap) ranges += otherR


I have checked this with claude code and it's indeed an issue that also affects the spark3 module. I think we should address this on a separate PR, fixing it on both modules.

wchevreuil · 2026-09-10T14:39:51Z

+        if (scanRanges.isEmpty && points.isEmpty) {
+          regions.map { region =>
+            val fullRange = Range(region)
+            HBaseInputPartition(region.index, Seq(fullRange), Seq.empty): InputPartition
+          }


Addressing on next commit.

wchevreuil · 2026-09-10T14:29:11Z

+  private def setStopRow(scan: Scan, bound: Bound): Scan = {
+    if (bound.inc) {
+      val incremented = Utils.incrementByteArray(bound.b)
+      if (incremented != null) scan.withStopRow(incremented)
+      else scan


Fixing in next commit.

+case class HBaseInputPartition(
+    index: Int,
+    scanRanges: Seq[Range],
+    points: Seq[Array[Byte]])
+    extends InputPartition


taklwu requested a balanced review from Copilot August 29, 2026 07:13

Copilot started reviewing on behalf of taklwu August 29, 2026 07:13 View session

Copilot AI reviewed Aug 29, 2026

View reviewed changes

taklwu requested changes Sep 1, 2026

View reviewed changes

wchevreuil requested review from taklwu and a balanced review from Copilot September 8, 2026 16:23

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Copilot started reviewing on behalf of wchevreuil September 9, 2026 11:02 View session

wchevreuil force-pushed the HBASE-30189 branch from d2bc359 to 31be97a Compare September 9, 2026 16:25

wchevreuil and others added 7 commits September 10, 2026 10:07

HBASE-30374 Update base docker image for hbase-connectors jenkins CI …

53b29ca

…job (apache#164) Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org> Signed-off-by: Dávid Paksy <paksyd@apache.org> Reviewed-by: Kevin Geiszler <kevin.j.geiszler@gmail.com>

Port pure utility classes with no Spark SQL dependency (apache#159)

7d72811

Co-authored-by: Claude Code (claude-opus-4-6) <no-reply@anthropic.com> Signed-off-by: Peter Somogyi <psomogyi@apache.org>

Port the JSON catalog parsing that maps HBase tables to Spark SQL sch…

a4661b8

…emas (apache#160) Co-authored-by: Claude Code (claude-opus-4-6) <no-reply@anthropic.com> Signed-off-by: Peter Somogyi <psomogyi@apache.org>

Implementing DataSource V2 Read Path

308fd07

Including javadoc on all new classes, plus test suites.

3914591

Addressing review comments

05b8103

addressing new round of reviews

2552079

wchevreuil force-pushed the HBASE-30189 branch from 31be97a to 2552079 Compare September 10, 2026 09:51

Merge branch 'HBASE-30189' into HBASE-30189

0e2969f

wchevreuil requested a balanced review from Copilot September 10, 2026 11:28

Copilot started reviewing on behalf of wchevreuil September 10, 2026 11:38 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Uh oh!

Conversation

wchevreuil commented Aug 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

taklwu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wchevreuil commented Sep 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Apache-HBase commented Sep 9, 2026

Uh oh!

Apache-HBase commented Sep 9, 2026

Uh oh!

Apache-HBase commented Sep 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants