Remove BuiltInPostgresType attribute reflection #5375
Conversation
NinoFloris
force-pushed
the
remove-attribute-reflection
branch
5 times, most recently
from
750aaa3 to
7a68fd6
Compare
|
Folding all data into the MinimalDatabaseInfo reduces our binary size by a small amount (4kb) and reduces our reliance on reflection, probably worth it. |
NinoFloris
changed the title
NinoFloris
force-pushed
the
remove-attribute-reflection
branch
from
7a68fd6 to
07a3fd5
Compare
roji
left a comment
roji
left a comment
There was a problem hiding this comment.
Pretty straightforward, thanks!
In the future, we can consider making PgMinimalDatabaseInfo a programmatic opt-in, rather than the current connection string. This would allow it to get trimmed if not used, and goes in the direction of at least moving non-environmental options out of the connection string.
| @@ -0,0 +1,21 @@ | |||
| namespace Npgsql.PostgresTypes; | |||
|
|
|||
| enum PostgresTypeKind | |||
There was a problem hiding this comment.
I personally prefer to keep the PgTypeKind naming for the succinctness.. I'd actually consider going in the other direction and rename other types prefixed with Postgres to Pg (e.g. PgMinimalDatabaseInfo)
There was a problem hiding this comment.
I renamed it to fit in with the other Postgres* types in here. I agree Pg is nice and succinct but we can't rename the others (since they're public) so I went with Postgres for consistency.
We can try but we're using it unconditionally as a bootstrap type catalog to go from datatypenames in the resolvers to pgtypeid oids. I think we'll always need such a minimal map available. |
NinoFloris
force-pushed
the
remove-attribute-reflection
branch
from
07a3fd5 to
e4ecf91
Compare
2 participants
Some work that I didn't want to put into #5123
Related to #4949.
Though not because of as reflection free mode is essentially unsupported.
It would expand our minimal set slightly too as previously we didn't import range array types or multirange array types, they weren't specified.