tanishqgandhi1908 commented on code in PR #7789:
URL: https://github.com/apache/texera/pull/7789#discussion_r3825960395
##########
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:
This is correct rather than a false positive. If a user's email really is
"datasets" and they own a dataset named after the second segment, then
/datasets/imdb/raw/movies.csv genuinely is a legacy unprefixed
path, and prefixing it to /dataset/datasets/imdb/raw/movies.csv is the
intended normalization — both forms parse to the same (owner, name, version,
file) tuple in FileResolver.
Excluding the plural prefix here would leave that path unprefixed instead,
which resolves only through the legacy fallback that this migration exists to
make removable (the TODO at FileResolver.scala:118).
On "remains falsely affected on every run": it isn't. Once rewritten the
value starts "/dataset/", so this arm's own left(fn, 9) guard rejects it and
the plural arm's regex doesn't match. Verified — a second run reports 0 rows.
--
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]
