Conversation
| import com.google.common.base.Preconditions; | ||
|
|
||
| @InternalApi | ||
| public class PostgreSQLStatementParser extends AbstractStatementParser { |
There was a problem hiding this comment.
woa, hand wrote parser. Nice job, we should also have this kind of parser at the backend.
There was a problem hiding this comment.
You could re-write the backend to Java and then add this parser :-)
| return getDoubleListInternal(columnIndex); | ||
| case NUMERIC: | ||
| return getBigDecimalListInternal(columnIndex); | ||
| case PG_NUMERIC: |
There was a problem hiding this comment.
PG does not support Struct, right? So, for now, we will not hit this PG_NUMERIC in Struct?
There was a problem hiding this comment.
PG currently does not support structs in the backend, so in that sense we won't see this coming from the backend. The client library however allows users to convert a row to a struct, and in that case we could encounter it:
thiagotnunes
left a comment
There was a problem hiding this comment.
I think we did not parameterize some integration tests with the pg dialect. Could you add the parameterization for the ITBatchReadTest?
| * Dialect#values()}. | ||
| */ | ||
| public @Nullable Dialect getDialect() { | ||
| return dialect; |
There was a problem hiding this comment.
Should it ever be null? Maybe we can say if null return UNSPECIFIED or GOOGLE_SQL_STANDARD?
There was a problem hiding this comment.
The DatabaseInfo(DatabaseId id, State state) constructor sets it to null (and also a lot of other fields that also could have had non-null values, like the encryption config), so I think we should keep it this way to keep it consistent with the other properties in the class.
|
Thanks for doing this! |
I've parameterized the |
Adds support for Cloud Spanner databases using the PostgreSQL dialect.