JingsongLi commented on code in PR #1134:
URL: https://github.com/apache/paimon-rust/pull/1134#discussion_r4290774973
##########
crates/integrations/datafusion/src/procedures.rs:
##########
@@ -709,6 +743,37 @@ async fn proc_create_tag_from_timestamp(
ok_result(ctx)
}
+async fn proc_create_tag_from_watermark(
+ ctx: &SessionContext,
+ catalog: &Arc<dyn Catalog>,
+ catalog_name: &str,
+ args: &HashMap<String, String>,
+) -> DFResult<DataFrame> {
+ let table = get_table(catalog, catalog_name, args).await?;
+ let tag_name = require_arg(args, "tag")?;
+ let watermark_str = require_arg(args, "watermark")?;
+ let watermark: i64 = watermark_str
+ .parse()
+ .map_err(|_| DataFusionError::Plan(format!("Invalid watermark:
'{watermark_str}'")))?;
+ let time_retained = args.get("time_retained").map(String::as_str);
+
+ let (sm, tm) = managers(&table);
+ if tm.tag_exists(tag_name).await.map_err(to_datafusion_error)? {
+ return Err(DataFusionError::Plan(format!(
+ "Tag '{tag_name}' already exists"
+ )));
+ }
+ let snapshot = later_or_equal_watermark_from_all(&sm, &tm, watermark)
Review Comment:
[P2] Resolve watermark candidates from authoritative REST metadata
This new entry point accepts REST tables, but `managers(&table)` creates a
filesystem-only SnapshotManager, and `later_or_equal_watermark` enumerates only
physical snapshot files. REST snapshots can legitimately exist only in the
catalog: `RESTSnapshotCommit::commit` sends the snapshot over HTTP without
publishing a snapshot file. I reproduced this with two real SQL-written Parquet
snapshots (watermarks 1000/2000), retained snapshot-1 on storage, and
snapshot-2 served only by the REST catalog. `SELECT id FROM paimon.test_db.t1
ORDER BY id` returns both rows, but `CALL sys.create_tag_from_watermark(table
=> 'test_db.t1', tag => 'checkpoint', watermark => 2000)` fails with `No
snapshot found with watermark >= 2000`; the call makes no `/snapshot` request.
Thus a readable REST table cannot use the advertised checkpoint operation.
Please resolve candidates through the catalog-aware metadata path, or
explicitly reject/document unsupported REST tables before selection, with this
SQL/HTTP
regression. Merely switching the helper to `table.snapshot_manager()` is
insufficient because its watermark search still lists storage files.
--
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]
