Skip to content

Commit cb5a390

Browse files
sbdchdsbdchd
authored
fix(parser): json index operator with alter table (#61)
Turns out `name` is an Option and that `expr` can be a more complicated object rather than just a string. Previously we didn't have any tests that exercised the `expr` property. I think the issues with clippy are related to: rust-lang/rust-clippy#2604 fixes: #59
1 parent 40d39a8 commit cb5a390

14 files changed

+158
-23
lines changed

‎.github/workflows/main.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ jobs:
7979
- name: Install Clippy
8080
run: rustup component add clippy
8181

82+
- name: Get Clippy Version
83+
run: cargo clippy --version
84+
8285
- name: Lint
8386
run: ./s/lint
8487

‎cli/src/main.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
#![allow(clippy::match_wildcard_for_single_variants)]
2+
#[allow(clippy::non_ascii_literal)]
3+
#[allow(clippy::cast_sign_loss)]
14
mod reporter;
25
mod subcommand;
36
use crate::reporter::{
@@ -84,7 +87,7 @@ fn main() {
8487
&mut handle,
8588
&opts.paths,
8689
is_stdin,
87-
dump_ast_kind,
90+
&dump_ast_kind,
8891
));
8992
} else {
9093
match check_files(&opts.paths, is_stdin, opts.stdin_filepath, opts.exclude) {

‎cli/src/reporter.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ pub fn dump_ast_for_paths<W: io::Write>(
5757
f: &mut W,
5858
paths: &[String],
5959
is_stdin: bool,
60-
dump_ast: DumpAstOption,
60+
dump_ast: &DumpAstOption,
6161
) -> Result<(), DumpAstError> {
6262
let mut process_dump_ast = |sql: &str| -> Result<(), DumpAstError> {
6363
match dump_ast {
@@ -193,8 +193,7 @@ fn fmt_gcc<W: io::Write>(
193193
.iter()
194194
.map(|v| {
195195
match v {
196-
ViolationMessage::Note(s) => s,
197-
ViolationMessage::Help(s) => s,
196+
ViolationMessage::Note(s) | ViolationMessage::Help(s) => s,
198197
}
199198
.to_string()
200199
})
@@ -308,7 +307,7 @@ pub fn pretty_violations(
308307
// 1-indexed
309308
let lineno = sql[..start].lines().count() + 1;
310309

311-
let content = &sql[start..start + len + 1];
310+
let content = &sql[start..=start + len];
312311

313312
// TODO(sbdchd): could remove the leading whitespace and comments to
314313
// get cleaner reports
@@ -741,7 +740,7 @@ SELECT 1;
741740
assert_debug_snapshot!(pretty_violations(violations, sql, filename));
742741
}
743742

744-
/// pretty_violations was slicing the SQL improperly, trimming off the first
743+
/// `pretty_violations` was slicing the SQL improperly, trimming off the first
745744
/// letter.
746745
#[test]
747746
fn test_trimming_sql_newlines() {

‎cli/src/subcommand.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,12 @@ fn get_github_private_key(
7878
github_private_key: Option<String>,
7979
github_private_key_base64: Option<String>,
8080
) -> Result<String, SquawkError> {
81-
match github_private_key {
82-
Some(private_key) => Ok(private_key),
83-
None => {
84-
let key = github_private_key_base64.ok_or(SquawkError::GithubPrivateKeyMissing)?;
85-
let bytes = base64::decode(key)?;
86-
Ok(String::from_utf8(bytes)?)
87-
}
81+
if let Some(private_key) = github_private_key {
82+
Ok(private_key)
83+
} else {
84+
let key = github_private_key_base64.ok_or(SquawkError::GithubPrivateKeyMissing)?;
85+
let bytes = base64::decode(key)?;
86+
Ok(String::from_utf8(bytes)?)
8887
}
8988
}
9089

‎github/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
#![allow(clippy::doc_markdown)]
2+
#![allow(clippy::missing_errors_doc)]
3+
#![allow(clippy::single_match_else)]
14
use jsonwebtoken::{Algorithm, EncodingKey, Header};
25
use log::info;
36
use reqwest::header::{ACCEPT, AUTHORIZATION};

‎parser/src/ast.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,9 @@ impl Default for SortByNulls {
164164
#[derive(Debug, Deserialize, Serialize)]
165165
pub struct IndexElem {
166166
/// name of attribute to index, or NULL
167-
name: String,
167+
name: Option<String>,
168168
/// expression to index, or NULL
169-
expr: Option<String>,
169+
expr: Option<Value>,
170170
/// name for index column; NULL = default
171171
indexcolname: Option<String>,
172172
/// name of collation; NIL = default

‎parser/src/parse.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,6 +1170,15 @@ ALTER TABLE foobar ALTER COLUMN value SET DEFAULT TO_JSON(false);
11701170
assert_debug_snapshot!(res);
11711171
}
11721172

1173+
#[test]
1174+
fn test_json_index_operator() {
1175+
let sql = r#"
1176+
CREATE INDEX CONCURRENTLY IF NOT EXISTS "idx_a_foo_bar" ON "a" ((foo->>'bar'));
1177+
"#;
1178+
let res = parse_sql_query(sql);
1179+
assert_debug_snapshot!(res);
1180+
}
1181+
11731182
#[test]
11741183
fn test_create_subscription_stmt() {
11751184
let sql = r#"

‎parser/src/snapshots/squawk_parser__parse__tests__adding_index_non_concurrently.snap

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ Ok(
1313
index_params: [
1414
IndexElem(
1515
IndexElem {
16-
name: "field_name",
16+
name: Some(
17+
"field_name",
18+
),
1719
expr: None,
1820
indexcolname: None,
1921
collation: None,
@@ -60,7 +62,9 @@ Ok(
6062
index_params: [
6163
IndexElem(
6264
IndexElem {
63-
name: "field_name",
65+
name: Some(
66+
"field_name",
67+
),
6468
expr: None,
6569
indexcolname: None,
6670
collation: None,

‎parser/src/snapshots/squawk_parser__parse__tests__json_index_operator.snap

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
---
2+
source: parser/src/parse.rs
3+
expression: res
4+
---
5+
Ok(
6+
[
7+
RawStmt(
8+
RawStmt {
9+
stmt: IndexStmt(
10+
IndexStmt {
11+
access_method: "btree",
12+
idxname: "idx_a_foo_bar",
13+
index_params: [
14+
IndexElem(
15+
IndexElem {
16+
name: None,
17+
expr: Some(
18+
Object({
19+
"A_Expr": Object({
20+
"kind": Number(
21+
0,
22+
),
23+
"lexpr": Object({
24+
"ColumnRef": Object({
25+
"fields": Array([
26+
Object({
27+
"String": Object({
28+
"str": String(
29+
"foo",
30+
),
31+
}),
32+
}),
33+
]),
34+
"location": Number(
35+
66,
36+
),
37+
}),
38+
}),
39+
"location": Number(
40+
69,
41+
),
42+
"name": Array([
43+
Object({
44+
"String": Object({
45+
"str": String(
46+
"->>",
47+
),
48+
}),
49+
}),
50+
]),
51+
"rexpr": Object({
52+
"A_Const": Object({
53+
"location": Number(
54+
72,
55+
),
56+
"val": Object({
57+
"String": Object({
58+
"str": String(
59+
"bar",
60+
),
61+
}),
62+
}),
63+
}),
64+
}),
65+
}),
66+
}),
67+
),
68+
indexcolname: None,
69+
collation: None,
70+
opclass: [],
71+
ordering: Default,
72+
nulls_ordering: Default,
73+
},
74+
),
75+
],
76+
relation: RangeVar(
77+
RangeVar {
78+
catalogname: None,
79+
schemaname: None,
80+
relname: "a",
81+
inh: true,
82+
relpersistence: "p",
83+
alias: None,
84+
location: 60,
85+
},
86+
),
87+
concurrent: true,
88+
unique: false,
89+
primary: false,
90+
isconstraint: false,
91+
deferrable: false,
92+
initdeferred: false,
93+
transformed: false,
94+
if_not_exists: true,
95+
table_space: None,
96+
},
97+
),
98+
stmt_location: 0,
99+
stmt_len: Some(
100+
79,
101+
),
102+
},
103+
),
104+
],
105+
)

‎parser/src/snapshots/squawk_parser__parse__tests__migration.snap

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ Ok(
2626
index_params: [
2727
IndexElem(
2828
IndexElem {
29-
name: "field_name",
29+
name: Some(
30+
"field_name",
31+
),
3032
expr: None,
3133
indexcolname: None,
3234
collation: None,
@@ -73,7 +75,9 @@ Ok(
7375
index_params: [
7476
IndexElem(
7577
IndexElem {
76-
name: "field_name",
78+
name: Some(
79+
"field_name",
80+
),
7781
expr: None,
7882
indexcolname: None,
7983
collation: None,

‎parser/src/snapshots/squawk_parser__parse__tests__parse_sql_create_index.snap

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ Ok(
1313
index_params: [
1414
IndexElem(
1515
IndexElem {
16-
name: "table_field",
16+
name: Some(
17+
"table_field",
18+
),
1719
expr: None,
1820
indexcolname: None,
1921
collation: None,

‎parser/src/snapshots/squawk_parser__parse__tests__parse_sql_create_index_concurrently.snap

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ Ok(
1313
index_params: [
1414
IndexElem(
1515
IndexElem {
16-
name: "table_field",
16+
name: Some(
17+
"table_field",
18+
),
1719
expr: None,
1820
indexcolname: None,
1921
collation: None,

‎parser/src/snapshots/squawk_parser__parse__tests__parsing_create_table.snap

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,9 @@ Ok(
502502
index_params: [
503503
IndexElem(
504504
IndexElem {
505-
name: "age",
505+
name: Some(
506+
"age",
507+
),
506508
expr: None,
507509
indexcolname: None,
508510
collation: None,

‎s/lint

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
set -eu
33

44
cargo fmt -- --check
5-
cargo clippy --all-targets --all-features -- -D clippy::nursery
5+
cargo clippy --all-targets --all-features -- -D clippy::pedantic

0 commit comments

Comments
 (0)