Skip to content

Conversation

@danielkweon
Copy link
Contributor

feat [properties] - backend soup endpoint for filter (sort coming soon)

@danielkweon danielkweon self-assigned this Jan 9, 2026
@danielkweon danielkweon requested review from a team as code owners January 9, 2026 20:05
@linear
Copy link

linear bot commented Jan 9, 2026

@github-actions
Copy link

github-actions bot commented Jan 9, 2026

@danielkweon danielkweon force-pushed the daniel/m-4974--feat-properties-backend-soup-endpoint-for-filter-and-sort branch 2 times, most recently from e4d4c5a to 3e4f1d1 Compare January 12, 2026 16:27
@claude
Copy link
Contributor

claude bot commented Jan 12, 2026

Code review

Issue: Missing .sqlx cache update

File: rust/cloud-storage/macro_db_client/src/document/preview.rs (line 50)

The SQL query was modified (changing updated_at to updated_at! to mark the field as non-nullable), but the .sqlx cache files were not updated.

According to the CLAUDE.md:

Any time you make changes to the SQL code in rust, you need to run just prepare_db in macro-api/cloud-storage/macro_db_client to update the .sqlx.

Resolution: Run just prepare_db from the macro-api/cloud-storage/macro_db_client directory to generate the .sqlx cache files.

- Add property_filters field to SoupRequest and SimpleSortRequest
- Extract property_filters in router before converting to EntityFilterAst
- Route to dynamic query when property_filters is not empty
- Build EXISTS subqueries for each property filter per entity type

Note: Frecency sort does not yet apply property filters.
- Optimize HasAll to use single @> check with array of all values
  instead of multiple AND checks for better performance
- Remove unused apply_filter function (not used in production)
- Remove redundant test_sql_apply_filter_uses_default_alias test
- Add push_has_all_check function for efficient HasAll queries
@danielkweon danielkweon force-pushed the daniel/m-4974--feat-properties-backend-soup-endpoint-for-filter-and-sort branch from 3e4f1d1 to 9ce18a5 Compare January 12, 2026 17:57
d."fileType" as file_type,
d.owner as "owner!",
d."updatedAt"::timestamptz as "updated_at",
d."updatedAt"::timestamptz as "updated_at!",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh weird, does this not require a regen of the queries?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants