Update record accessibility check

[acrellin]

This commit https://github.com/skyportal/skyportal/pull/1982/commits/091263665c28c78a19e305676f7b2b0ec590bf79 causes is_accessible_by to error for StreamUser records without this change

[dannygoldstein]

I'm not sure this should ever be necessary ?

[dannygoldstein]

Proposing an alternative ...

[dannygoldstein]

Actually, before I do, I am having trouble understanding this error case. Could you help me wrap my head around it?

If I do, in postgres,

skyportal=# select count(*) > 0 from photometry where false;

the result is

 ?column?
----------
 f
(1 row)

can you show the SQL query that results in this being none (0 rows in SQL output)? I am having trouble understanding how it can be produced

[acrellin]

@dannygoldstein did you see the linked commit? https://github.com/skyportal/skyportal/commit/091263665c28c78a19e305676f7b2b0ec590bf79

With that commit, is_accessible_by errors for StreamUser records

[dannygoldstein]

I couldn’t pull that commit, seems like it is not part of the repo ?

---

From: Ari Crellin-Quick @.> Sent: Wednesday, May 19, 2021 9:51:08 PM To: cesium-ml/baselayer @.> Cc: Danny Goldstein @.>; Mention @.> Subject: Re: [cesium-ml/baselayer] Update record accessibility check (#227)

@dannygoldsteinhttps://github.com/dannygoldstein did you see the linked commit? @.***https://github.com/skyportal/skyportal/commit/091263665c28c78a19e305676f7b2b0ec590bf79

With that commit, is_accessible_by errors for StreamUser records

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/cesium-ml/baselayer/pull/227#issuecomment-844690488, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAVEFYCMUNVDX6SAUTPODVTTOSILZANCNFSM44ZLS7HA.

[acrellin]

Yeah, it's not merged to SP yet -- you'll have to fetch my fork first

[acrellin]

@dannygoldstein see https://github.com/skyportal/skyportal/pull/1982

[kmshin1397]

@dannygoldstein Here is the SQL generated by SQLAlchemy which generated the 0 row output:

SELECT          count(*) > 0 AS delete_ok
FROM            stream_users
JOIN            users
ON              users.id = stream_users.user_id
LEFT OUTER JOIN (group_users AS group_users_1
              JOIN            groups
              ON              groups.id = group_users_1.group_id)
ON              users.id = group_users_1.user_id
LEFT OUTER JOIN group_streams
ON              group_streams.group_id = groups.id
AND             group_streams.stream_id = stream_users.stream_id
WHERE           false
AND             stream_users.id = 1
GROUP BY        stream_users.id
HAVING          bool_and(group_streams.stream_id IS NULL)

[kmshin1397]

Looks like the GROUP BY/HAVING is what causes it to return 0 rows instead of the false (corresponding to these lines)

[acrellin]

Thanks for digging into this a bit more deeply, @kmshin1397

[acrellin]

@dannygoldstein wanna take another look / offer any feedback before we merge? See Kyung's explanatory comments above.

[dannygoldstein]

I would consider moving the inner query to a subquery and selecting count(*) > 0 from that

---

From: Ari Crellin-Quick @.> Sent: Tuesday, May 25, 2021 12:17:56 PM To: cesium-ml/baselayer @.> Cc: Danny Goldstein @.>; Mention @.> Subject: Re: [cesium-ml/baselayer] Update record accessibility check (#227)

@dannygoldsteinhttps://github.com/dannygoldstein wanna take another look / offer any feedback before we merge? See Kyung's explanatory comments above.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/cesium-ml/baselayer/pull/227#issuecomment-848195124, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAVEFYACNFP5I5PBBU6ZXJTTPPZWJANCNFSM44ZLS7HA.

[acrellin]

Closing as no longer needed