slices.network / quickslice
Auto-indexing service and GraphQL API for AT Protocol Records quickslice.slices.network/
atproto gleam graphql
fork atom

fix: filtered queries with Postgres #2

merged opened by trezy.codes targeting main from [deleted fork]: main

The build_where_sql() function always started parameter numbering from 1, regardless of how many parameters were already bound before calling it. This caused placeholder collisions on Postgres (which uses numbered $1, $2 placeholders) but worked on SQLite (which uses ? and ignores indices).

For example, a filtered query would generate: WHERE record.collection = $1 AND json->>'uri' = $1 Instead of: WHERE record.collection = $1 AND json->>'uri' = $2

This follows the same pattern as the pagination fix in commit d2dd4a0.

Fixes #7

Labels

None yet.

assignee

None yet.

Participants 1
AT URI
at://did:plc:4jrld6fwpnwqehtce56qshzv/sh.tangled.repo.pull/3mcy5xzdwwz22
+88 -57
Diff #0
+6 -1
server/src/database/queries/aggregates.gleam
··· 74 74 True -> #(mut_where_parts, mut_bind_values) 75 75 False -> { 76 76 let #(where_sql, where_params) = 77 - where_clause.build_where_sql(exec, wc, True) 77 + where_clause.build_where_sql( 78 + exec, 79 + wc, 80 + True, 81 + list.length(mut_bind_values) + 1, 82 + ) 78 83 let new_where = list.append(mut_where_parts, [where_sql]) 79 84 let new_binds = list.append(mut_bind_values, where_params) 80 85 #(new_where, new_binds)
+3 -1
server/src/database/queries/where_clause.gleam
··· 364 364 /// Builds WHERE clause SQL from a WhereClause 365 365 /// Returns tuple of (sql_string, parameters) 366 366 /// use_table_prefix: if True, prefixes table columns with "record." for joins 367 + /// start_index: the starting parameter index (1-based) for placeholders 367 368 pub fn build_where_sql( 368 369 exec: Executor, 369 370 clause: WhereClause, 370 371 use_table_prefix: Bool, 372 + start_index: Int, 371 373 ) -> #(String, List(Value)) { 372 374 case is_clause_empty(clause) { 373 375 True -> #("", []) 374 376 False -> { 375 377 let #(sql_parts, params, _) = 376 - build_where_clause_internal(exec, clause, use_table_prefix, 1) 378 + build_where_clause_internal(exec, clause, use_table_prefix, start_index) 377 379 let sql = string.join(sql_parts, " AND ") 378 380 #(sql, params) 379 381 }
+28 -4
server/src/database/repositories/records.gleam
··· 472 472 case where_clause.is_clause_empty(wc) { 473 473 True -> #(mut_where_parts, mut_bind_values) 474 474 False -> { 475 + // Start index is 2 because $1 is used for collection 475 476 let #(where_sql, where_params) = 476 - where_clause.build_where_sql(exec, wc, needs_actor_join) 477 + where_clause.build_where_sql( 478 + exec, 479 + wc, 480 + needs_actor_join, 481 + list.length(mut_bind_values) + 1, 482 + ) 477 483 let new_where = list.append(mut_where_parts, [where_sql]) 478 484 let new_binds = list.append(mut_bind_values, where_params) 479 485 #(new_where, new_binds) ··· 684 690 case where_clause.is_clause_empty(wc) { 685 691 True -> #(mut_where_parts, mut_bind_values) 686 692 False -> { 693 + // Start index accounts for prior bind values ($1 = collection) 687 694 let #(where_sql, where_params) = 688 - where_clause.build_where_sql(exec, wc, needs_actor_join) 695 + where_clause.build_where_sql( 696 + exec, 697 + wc, 698 + needs_actor_join, 699 + list.length(mut_bind_values) + 1, 700 + ) 689 701 let new_where = list.append(mut_where_parts, [where_sql]) 690 702 let new_binds = list.append(mut_bind_values, where_params) 691 703 #(new_where, new_binds) ··· 924 936 // Add where clause conditions if present 925 937 let #(with_where_parts, with_where_values) = case wc { 926 938 Some(clause) -> { 939 + // Start index accounts for prior bind values ($1 = collection, $2 = parent_uri, $3 = parent_uri) 927 940 let #(where_sql, where_params) = 928 - where_clause.build_where_sql(exec, clause, False) 941 + where_clause.build_where_sql( 942 + exec, 943 + clause, 944 + False, 945 + list.length(base_bind_values) + 1, 946 + ) 929 947 case where_sql { 930 948 "" -> #(base_where_parts, base_bind_values) 931 949 _ -> #( ··· 1281 1299 // Add where clause conditions if present 1282 1300 let #(with_where_parts, with_where_values) = case wc { 1283 1301 Some(clause) -> { 1302 + // Start index accounts for prior bind values ($1 = did, $2 = collection) 1284 1303 let #(where_sql, where_params) = 1285 - where_clause.build_where_sql(exec, clause, False) 1304 + where_clause.build_where_sql( 1305 + exec, 1306 + clause, 1307 + False, 1308 + list.length(base_bind_values) + 1, 1309 + ) 1286 1310 case where_sql { 1287 1311 "" -> #(base_where_parts, base_bind_values) 1288 1312 _ -> #(
+18 -18
server/test/where_edge_cases_test.gleam
··· 24 24 let clause = 25 25 where_clause.WhereClause(conditions: dict.new(), and: None, or: None) 26 26 27 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 27 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 28 28 29 29 sql |> should.equal("") 30 30 list.length(params) |> should.equal(0) ··· 51 51 or: None, 52 52 ) 53 53 54 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 54 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 55 55 56 56 // Should produce no SQL since all conditions are None 57 57 sql |> should.equal("") ··· 79 79 or: None, 80 80 ) 81 81 82 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 82 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 83 83 84 84 // Empty IN list should produce no SQL 85 85 sql |> should.equal("") ··· 91 91 let clause = 92 92 where_clause.WhereClause(conditions: dict.new(), and: Some([]), or: None) 93 93 94 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 94 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 95 95 96 96 sql |> should.equal("") 97 97 list.length(params) |> should.equal(0) ··· 102 102 let clause = 103 103 where_clause.WhereClause(conditions: dict.new(), and: None, or: Some([])) 104 104 105 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 105 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 106 106 107 107 sql |> should.equal("") 108 108 list.length(params) |> should.equal(0) ··· 141 141 or: None, 142 142 ) 143 143 144 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 144 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 145 145 146 146 // Should use parameterized query with json_extract for non-table columns 147 147 sql |> should.equal("json_extract(json, '$.username') = ?") ··· 178 178 or: None, 179 179 ) 180 180 181 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 181 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 182 182 183 183 // Should use LIKE with parameterized value and json_extract for non-table fields 184 184 sql ··· 237 237 ) 238 238 239 239 let #(sql_large, params_large) = 240 - where_clause.build_where_sql(exec, clause_large, False) 240 + where_clause.build_where_sql(exec, clause_large, False, 1) 241 241 let #(sql_small, params_small) = 242 - where_clause.build_where_sql(exec, clause_small, False) 242 + where_clause.build_where_sql(exec, clause_small, False, 1) 243 243 244 244 // count is a JSON field, not a table column 245 245 sql_large |> should.equal("json_extract(json, '$.count') = ?") ··· 270 270 or: None, 271 271 ) 272 272 273 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 273 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 274 274 275 275 // Empty string is still valid - should use json_extract for non-table columns 276 276 sql |> should.equal("json_extract(json, '$.name') = ?") ··· 308 308 or: None, 309 309 ) 310 310 311 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 311 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 312 312 313 313 sql |> should.equal("json_extract(json, '$.text') = ?") 314 314 list.length(params) |> should.equal(1) ··· 364 364 or: None, 365 365 ) 366 366 367 - let #(sql, params) = where_clause.build_where_sql(exec, level5, False) 367 + let #(sql, params) = where_clause.build_where_sql(exec, level5, False, 1) 368 368 369 369 // With single condition at each level, no extra parentheses needed 370 370 // The implementation correctly doesn't add unnecessary parentheses for single conditions ··· 425 425 or: None, 426 426 ) 427 427 428 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 428 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 429 429 430 430 // Should only include non-empty conditions 431 431 list.length(params) |> should.equal(2) ··· 570 570 or: None, 571 571 ) 572 572 573 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 573 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 574 574 575 575 // Should combine both operators with json_extract and CAST for numeric comparison 576 576 sql ··· 602 602 or: None, 603 603 ) 604 604 605 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 605 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 606 606 607 607 // Should apply both (though logically this might not make sense) 608 608 list.length(params) |> should.equal(3) ··· 642 642 or: None, 643 643 ) 644 644 645 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 645 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 646 646 647 647 // Should generate correct number of placeholders 648 648 list.length(params) |> should.equal(100) ··· 683 683 or: None, 684 684 ) 685 685 686 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 686 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 687 687 688 688 // Should use json_extract for dotted field names 689 689 sql ··· 721 721 or: None, 722 722 ) 723 723 724 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 724 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 725 725 726 726 // Should use json_extract even for non-dotted field names 727 727 let expected = "json_extract(json, '$." <> field_name <> "') = ?"
+33 -33
server/test/where_sql_builder_test.gleam
··· 21 21 pub fn build_where_empty_clause_test() { 22 22 let exec = get_test_exec() 23 23 let clause = where_clause.empty_clause() 24 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 24 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 25 25 26 26 sql |> should.equal("") 27 27 list.length(params) |> should.equal(0) ··· 49 49 or: None, 50 50 ) 51 51 52 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 52 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 53 53 54 54 sql |> should.equal("collection = ?") 55 55 list.length(params) |> should.equal(1) ··· 81 81 or: None, 82 82 ) 83 83 84 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 84 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 85 85 86 86 sql |> should.equal("did IN (?, ?, ?)") 87 87 list.length(params) |> should.equal(3) ··· 109 109 or: None, 110 110 ) 111 111 112 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 112 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 113 113 114 114 sql |> should.equal("indexed_at > ?") 115 115 list.length(params) |> should.equal(1) ··· 137 137 or: None, 138 138 ) 139 139 140 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 140 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 141 141 142 142 // Now includes CAST for numeric comparisons on JSON fields 143 143 sql |> should.equal("CAST(json_extract(json, '$.year') AS INTEGER) >= ?") ··· 166 166 or: None, 167 167 ) 168 168 169 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 169 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 170 170 171 171 sql |> should.equal("indexed_at < ?") 172 172 list.length(params) |> should.equal(1) ··· 194 194 or: None, 195 195 ) 196 196 197 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 197 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 198 198 199 199 // Now includes CAST for numeric comparisons on JSON fields 200 200 sql |> should.equal("CAST(json_extract(json, '$.count') AS INTEGER) <= ?") ··· 223 223 or: None, 224 224 ) 225 225 226 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 226 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 227 227 228 228 // Should combine both conditions with AND 229 229 sql |> should.equal("indexed_at > ? AND indexed_at < ?") ··· 267 267 or: None, 268 268 ) 269 269 270 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 270 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 271 271 272 272 // Order might vary due to dict, but should have AND 273 273 should.be_true(string.contains(sql, "AND")) ··· 300 300 or: None, 301 301 ) 302 302 303 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 303 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 304 304 305 305 sql |> should.equal("json_extract(json, '$.text') = ?") 306 306 list.length(params) |> should.equal(1) ··· 328 328 or: None, 329 329 ) 330 330 331 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 331 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 332 332 333 333 sql |> should.equal("json_extract(json, '$.user.name') = ?") 334 334 list.length(params) |> should.equal(1) ··· 356 356 or: None, 357 357 ) 358 358 359 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 359 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 360 360 361 361 sql |> should.equal("json_extract(json, '$.metadata.tags.0') = ?") 362 362 list.length(params) |> should.equal(1) ··· 384 384 or: None, 385 385 ) 386 386 387 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 387 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 388 388 389 389 // Now includes CAST for numeric comparisons on JSON fields 390 390 sql ··· 431 431 or: None, 432 432 ) 433 433 434 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 434 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 435 435 436 436 // Should have both table column and JSON extract with CAST for numeric comparison 437 437 should.be_true(string.contains(sql, "collection = ?")) ··· 467 467 or: None, 468 468 ) 469 469 470 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 470 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 471 471 472 472 sql 473 473 |> should.equal( ··· 498 498 or: None, 499 499 ) 500 500 501 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 501 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 502 502 503 503 sql |> should.equal("uri LIKE '%' || ? || '%' COLLATE NOCASE") 504 504 list.length(params) |> should.equal(1) ··· 526 526 or: None, 527 527 ) 528 528 529 - let #(sql, _params) = where_clause.build_where_sql(exec, clause, False) 529 + let #(sql, _params) = where_clause.build_where_sql(exec, clause, False, 1) 530 530 531 531 // SQL should be generated (actual escaping would be handled by the parameter binding) 532 532 should.be_true(string.contains(sql, "LIKE")) ··· 570 570 or: None, 571 571 ) 572 572 573 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 573 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 574 574 575 575 // Should have both LIKE clauses 576 576 should.be_true(string.contains(sql, "LIKE")) ··· 600 600 or: None, 601 601 ) 602 602 603 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 603 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 604 604 605 605 // Should have both LIKE and > operator 606 606 should.be_true(string.contains(sql, "LIKE")) ··· 665 665 or: None, 666 666 ) 667 667 668 - let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False) 668 + let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False, 1) 669 669 670 670 // Should have both conditions AND'ed together with parentheses 671 671 should.be_true(string.contains(sql, "collection = ?")) ··· 721 721 or: None, 722 722 ) 723 723 724 - let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False) 724 + let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False, 1) 725 725 726 726 // Should have both root condition and nested condition 727 727 should.be_true(string.contains(sql, "collection = ?")) ··· 785 785 or: None, 786 786 ) 787 787 788 - let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False) 788 + let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False, 1) 789 789 790 790 // Should have both conditions 791 791 should.be_true(string.contains(sql, "artist")) ··· 865 865 or: None, 866 866 ) 867 867 868 - let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False) 868 + let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False, 1) 869 869 870 870 // Should have all three conditions 871 871 should.be_true(string.contains(sql, "collection = ?")) ··· 931 931 or: Some([clause1, clause2]), 932 932 ) 933 933 934 - let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False) 934 + let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False, 1) 935 935 936 936 // Should have both conditions OR'ed together 937 937 should.be_true(string.contains(sql, "artist")) ··· 1026 1026 or: None, 1027 1027 ) 1028 1028 1029 - let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False) 1029 + let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False, 1) 1030 1030 1031 1031 // Should have proper precedence: (artist LIKE OR genre =) AND year >= 1032 1032 should.be_true(string.contains(sql, "OR")) ··· 1172 1172 or: None, 1173 1173 ) 1174 1174 1175 - let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False) 1175 + let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False, 1) 1176 1176 1177 1177 // Should have both OR and AND with proper nesting 1178 1178 should.be_true(string.contains(sql, "OR")) ··· 1260 1260 or: Some([clause1, clause2, clause3]), 1261 1261 ) 1262 1262 1263 - let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False) 1263 + let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False, 1) 1264 1264 1265 1265 // Should have all three OR'ed together 1266 1266 should.be_true(string.contains(sql, "OR")) ··· 1292 1292 or: None, 1293 1293 ) 1294 1294 1295 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 1295 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 1296 1296 1297 1297 sql |> should.equal("json_extract(json, '$.replyParent') IS NULL") 1298 1298 list.length(params) |> should.equal(0) ··· 1320 1320 or: None, 1321 1321 ) 1322 1322 1323 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 1323 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 1324 1324 1325 1325 sql |> should.equal("json_extract(json, '$.replyParent') IS NOT NULL") 1326 1326 list.length(params) |> should.equal(0) ··· 1348 1348 or: None, 1349 1349 ) 1350 1350 1351 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 1351 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 1352 1352 1353 1353 sql |> should.equal("cid IS NULL") 1354 1354 list.length(params) |> should.equal(0) ··· 1376 1376 or: None, 1377 1377 ) 1378 1378 1379 - let #(sql, params) = where_clause.build_where_sql(exec, clause, False) 1379 + let #(sql, params) = where_clause.build_where_sql(exec, clause, False, 1) 1380 1380 1381 1381 sql |> should.equal("uri IS NOT NULL") 1382 1382 list.length(params) |> should.equal(0) ··· 1404 1404 or: None, 1405 1405 ) 1406 1406 1407 - let #(sql, params) = where_clause.build_where_sql(exec, clause, True) 1407 + let #(sql, params) = where_clause.build_where_sql(exec, clause, True, 1) 1408 1408 1409 1409 sql |> should.equal("json_extract(record.json, '$.text') IS NULL") 1410 1410 list.length(params) |> should.equal(0) ··· 1464 1464 or: None, 1465 1465 ) 1466 1466 1467 - let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False) 1467 + let #(sql, params) = where_clause.build_where_sql(exec, root_clause, False, 1) 1468 1468 1469 1469 should.be_true(string.contains(sql, "IS NULL")) 1470 1470 should.be_true(string.contains(sql, "LIKE"))

History

1 round 1 comment
sign up or login to add to the discussion
trezy.codes submitted #0
1 commit
expand
eb821859
fix: filtered queries with Postgres
expand 1 comment
trezy.codes

Confirmed on my local build that this does solve the general issue with filtering (as long as sortBy uses a generated field).

pull request successfully merged