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.

Implemented the "preferredLocations" method in the InputPartition.

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

Apache-HBase · 2026-09-11T13:28:41Z

(!) 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/11/console in case of problems.

Copilot

🟡 Changes recommended

The read path has unresolved serialization, filter encoding, partial-result, configuration, and Catalyst conversion defects.

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

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

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

A positive HBase Scan batch size allows one logical row to be returned as multiple partial Results. This reader treats every Result as a complete Spark row, so hbase.spark.query.batchsize can produce duplicate rows with nulls for cells delivered in another batch. Either coalesce partial results (using mayHaveMoreCellsInRow) before exposing them or avoid setting a cell batch size.

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

Complex values are still returned in external Scala form here. SchemaConverters.createConverterToSQL produces Row, Seq, and Map for struct/array/map fields, but GenericInternalRow requires InternalRow, ArrayData, and MapData; reading an Avro-backed complex column will therefore fail or expose an invalid Catalyst value. Use Spark's recursive Catalyst converter rather than handling only scalar special cases.

      case dt: DecimalType =>
        Decimal(value.asInstanceOf[java.math.BigDecimal], dt.precision, dt.scale)
      case _ => value

spark4/hbase-spark4/src/main/scala/org/apache/hadoop/hbase/spark/datasources/HBaseScanBuilder.scala:60

This ignores the documented global SparkConf fallback (HBaseSparkConf says settings may be global or per-table). Consequently spark.conf.set("hbase.spark.pushdown.columnfilter", "false") has no effect unless the option is repeated on every read, and filters are still reported as fully pushed. Preserve per-read precedence but fall back to the active SparkConf as the Spark 3 path does.

    val usePushDown = properties
      .get(HBaseSparkConf.PUSHDOWN_COLUMN_FILTER)
      .map(_.toBoolean)
      .getOrElse(HBaseSparkConf.DEFAULT_PUSHDOWN_COLUMN_FILTER)

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

Apache-HBase · 2026-09-11T15:28:40Z

(!) 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/12/console in case of problems.

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 18 out of 18 changed files in this pull request and generated 3 comments.

wchevreuil · 2026-09-14T10:53:24Z

+        val endKeys = keys.getSecond
+
+        val regions = startKeys.zip(endKeys).zipWithIndex.map { case ((start, end), idx) =>
+          HBaseRegion(idx, Some(start), Some(end),


This isn't a valid issue. The potential problem is addressed in Range.apply(region) code, where empty keys are converted to None (see Bound.scala lines #36-44).

wchevreuil · 2026-09-14T11:14:42Z

+    if (compareRange(lowerBound, other.lowerBound) < 0 ||
+      compareRange(upperBound, other.upperBound) < 0) {
+      leftRange = this
+      rightRange = other
+    } else {
+      leftRange = other
+      rightRange = this
+    }


This would still pass the overlaps check which then merges the ScanRanges results accordingly. It copies the logic from spark3 module in DefaultSource.scala. I would suggest optimisation work to be done separately.

wchevreuil · 2026-09-14T11:20:12Z

+  private val resultIterator: Iterator[Result] = {
+    val scanIterators = partition.scanRanges.map { range =>
+      val scanner = buildScanner(range)
+      scanners += scanner
+      scannerToIterator(scanner)
+    }
+    val getIterator = if (partition.points.nonEmpty) {
+      buildGets(partition.points)
+    } else {
+      Iterator.empty
+    }
+    scanIterators.foldLeft(Iterator.empty: Iterator[Result])(_ ++ _) ++ getIterator
+  }


This is pre-existing spark3 module logic, and in practice, scan ranges intersections are very few, so this doesn't present a real performance problem.

wchevreuil · 2026-09-14T11:24:25Z

Hey @taklwu , I believe I have now addressed all copilot reviews that were indeed valid. Can you give an extra look into this?

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

addressing another round of reviews

313e1c5

wchevreuil requested a balanced review from Copilot September 11, 2026 13:25

Copilot started reviewing on behalf of wchevreuil September 11, 2026 13:26 View session

Copilot AI reviewed Sep 11, 2026

View reviewed changes

Comment thread ...4/src/main/scala/org/apache/hadoop/hbase/spark/datasources/HBasePartitionReaderFactory.scala

Comment thread ...hbase-spark4/src/main/scala/org/apache/hadoop/hbase/spark/datasources/HBaseScanBuilder.scala

making SerDes serialisable

af2e379

wchevreuil requested a balanced review from Copilot September 11, 2026 15:21

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

Copilot AI reviewed Sep 11, 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!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Apache-HBase commented Sep 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (3)

Uh oh!

Uh oh!

Uh oh!

Apache-HBase commented Sep 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment