JingsongLi commented on code in PR #8475:
URL: https://github.com/apache/paimon/pull/8475#discussion_r3527338164
##########
paimon-core/src/main/java/org/apache/paimon/globalindex/IndexedSplitRecordReader.java:
##########
@@ -59,17 +59,23 @@ public ScoreRecordIterator<InternalRow> readBatch() throws
IOException {
return new ScoreRecordIterator<InternalRow>() {
private float score = Float.NaN;
+ private long rowId;
@Override
public float returnedScore() {
return score;
}
+ @Override
+ public long returnedRowId() {
+ return rowId;
+ }
+
@Override
public InternalRow next() throws IOException {
InternalRow row = iterator.next();
if (row != null && rowIdToScore != null) {
- Long rowId = row.getLong(rowIdIndex);
+ this.rowId = row.getLong(rowIdIndex);
Review Comment:
Since `returnedRowId()` is now independent metadata, this assignment should
not be gated by `rowIdToScore != null`. If an indexed split has row ranges but
no scores, every row would expose the default/stale `0` row id. Please set
`rowId` whenever `row != null && rowIdIndex >= 0`, and only guard the score
lookup with `rowIdToScore != null`.
##########
paimon-spark/paimon-spark-ut/src/test/java/org/apache/paimon/spark/SparkMultimodalITCase.java:
##########
@@ -148,14 +148,42 @@ public void testVector(@TempDir java.nio.file.Path
tempDir) throws IOException {
"select gid, sid, embs from
vector_search('my_db1.vector_test', 'embs', array(1.0f, 2.0f, 3.0f, 4.0f), 5)
where date = '20260420'")
.collectAsList();
assertThat(rows).hasSize(5);
+
+ // **vector search with row id */
+ String vectorSearchWithRowIdSql =
+ "select gid, sid, embs, _row_id AS _row_id "
+ + "from vector_search('my_db1.vector_test', 'embs',
array(1.0f, 2.0f, 3.0f, 4.0f), 5) "
+ + "where date = '20260420'";
+ Dataset<Row> df = spark.sql(vectorSearchWithRowIdSql);
+ assertThat(df.columns()).hasSize(4);
+ assertThat(df.columns()).contains("_row_id");
+ rows = df.collectAsList();
+ assertThat(rows).hasSize(5);
+ assertThat(rows.stream().noneMatch(row -> row.isNullAt(3))).isTrue();
Review Comment:
This only checks `_row_id` is non-null, so an implementation that returns
the same wrong row id for every result would still pass. Please compare `(gid,
_row_id)` from `vector_search(...)` with the base table’s `(gid, _row_id)`
rows, and consider adding the same assertion for lateral vector search.
--
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]
