slbotbm commented on code in PR #3196:
URL: https://github.com/apache/iggy/pull/3196#discussion_r3242203840
##########
core/connectors/sources/postgres_source/src/lib.rs:
##########
Review Comment:
The current code matches against wrong literals, which means that everything
is coerced into a string.
Current literals (wrong):
```
- _BOOL
- _INT2
- _INT4
- _INT8
- _FLOAT4
- _FLOAT8
- _TEXT
- _VARCHAR
- _CHAR
- _BPCHAR
- _UUID
- _JSON
- _JSONB
```
Correct literals:
```
- BOOL[]
- INT2[]
- INT4[]
- INT8[]
- FLOAT4[]
- FLOAT8[]
- TEXT[]
- VARCHAR[]
- CHAR[]
- UUID[]
- JSON[]
- JSONB[]
```
##########
core/connectors/sources/postgres_source/src/lib.rs:
##########
@@ -995,10 +1021,130 @@ impl PostgresSource {
})
.unwrap_or(serde_json::Value::Null))
}
- _ => {
- let value: Option<String> = row
+ "_BOOL" => {
+ let value: Option<Vec<bool>> = row
+ .try_get(column_index)
+ .map_err(|_| Error::InvalidRecord)?;
+ Ok(value
+ .map(|arr| {
+ serde_json::Value::Array(
+
arr.into_iter().map(serde_json::Value::Bool).collect(),
+ )
+ })
+ .unwrap_or(serde_json::Value::Null))
+ }
+ "_INT2" => {
+ let value: Option<Vec<i16>> = row
Review Comment:
For arms decoding `Option<Vec<T>>`, the code assumes every element is
non-null, but postgresql can contain null elements. Thus, if a null value is
supplied to the current code, sqlx will fail to decode it.
##########
core/connectors/sources/postgres_source/src/lib.rs:
##########
Review Comment:
Also, the code treats BPCHAR as a possible option, but sqlx reports the
display name CHAR[], so the BPCHAR-style array match can never fire.
##########
core/connectors/sources/postgres_source/src/lib.rs:
##########
@@ -995,10 +1021,130 @@ impl PostgresSource {
})
.unwrap_or(serde_json::Value::Null))
}
- _ => {
- let value: Option<String> = row
+ "_BOOL" => {
+ let value: Option<Vec<bool>> = row
+ .try_get(column_index)
+ .map_err(|_| Error::InvalidRecord)?;
+ Ok(value
+ .map(|arr| {
+ serde_json::Value::Array(
+
arr.into_iter().map(serde_json::Value::Bool).collect(),
+ )
+ })
+ .unwrap_or(serde_json::Value::Null))
+ }
+ "_INT2" => {
+ let value: Option<Vec<i16>> = row
Review Comment:
Something like `Option<Vec<Option<T>>>` could mitigate this.
--
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]
