Copilot commented on code in PR #7789:
URL: https://github.com/apache/texera/pull/7789#discussion_r3825775239
##########
sql/updates/36.sql:
##########
@@ -59,12 +67,22 @@ BEGIN
) f
WHERE jsonb_typeof(w.content::jsonb -> 'operators') = 'array'
AND (
- (f.fn IS NOT NULL AND left(f.fn, 10) <> '/datasets/'
+ (f.fn ~ '^/datasets/'
+ AND EXISTS (SELECT 1 FROM dataset d JOIN "user" u ON
d.owner_uid = u.uid
+ WHERE u.email = split_part(ltrim(f.fn, '/'), '/',
2)
+ AND d.name = split_part(ltrim(f.fn, '/'), '/',
3)))
+ OR
+ (f.dvp ~ '^/datasets/'
+ AND EXISTS (SELECT 1 FROM dataset d JOIN "user" u ON
d.owner_uid = u.uid
+ WHERE u.email = split_part(ltrim(f.dvp, '/'),
'/', 2)
+ AND d.name = split_part(ltrim(f.dvp, '/'),
'/', 3)))
+ OR
+ (f.fn IS NOT NULL AND left(f.fn, 9) <> '/dataset/'
Review Comment:
The “unprefixed” filter still accepts `/datasets/...`. If the intended
owner/name lookup for a plural path fails but a local admin/user with email
`datasets` owns a dataset named after the second segment, this row remains
falsely affected on every run. Explicitly exclude the legacy plural prefix here
(and in the matching transform).
This issue also appears in the following locations of the same file:
- line 106
- line 177
- line 203
##########
sql/updates/36.sql:
##########
@@ -23,20 +23,28 @@ SET search_path TO texera_db;
BEGIN;
--- The file resolver now requires an explicit resource-type prefix on dataset
--- logical paths (/datasets/ownerEmail/datasetName/versionName/...) so other
--- resource types (e.g. models) can be told apart by the prefix. Existing
--- workflows store unprefixed dataset paths inside workflow.content and
--- workflow_version.content, in two operator properties:
+-- The file resolver requires an explicit resource-type prefix on dataset
logical
Review Comment:
Changing the contents of already-registered changeSet 36 will make Liquibase
4.29 reject any database that previously applied the version currently on
`main` because its stored checksum no longer matches. Allowing the old checksum
would still skip this new normalization, leaving plural paths incompatible with
the new code. Preserve changeSet 36 and add a new post-37 normalization
changeSet instead.
--
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]
