NEOS-969: filter out defaults #1843
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1843 +/- ##
==========================================
- Coverage 17.15% 10.98% -6.18%
==========================================
Files 214 119 -95
Lines 45003 37666 -7337
==========================================
- Hits 7721 4136 -3585
+ Misses 36317 32985 -3332
+ Partials 965 545 -420 ☔ View full report in Codecov by Sentry. |
| hasDefault := false | ||
| if col.ColumnDefault != nil { | ||
| // ColumnDefault is an interface we need to check the value to see if it's set or not | ||
| value := reflect.ValueOf(col.ColumnDefault) | ||
| hasDefault = !value.IsZero() | ||
| } | ||
|
|
There was a problem hiding this comment.
If you go into the system.sql file for postgres queries, there are two queries in there that can be updated that will make all of this code go away.
Look for this block (shows up twice):
COALESCE(
pg_catalog.pg_get_expr(d.adbin, d.adrelid),
''
) AS column_default,
That is the bit that calculates this column. However, sqlc isn't smart enough to know the type, so we just need to cast it to text like so:
COALESCE(
pg_catalog.pg_get_expr(d.adbin, d.adrelid),
''
)::text AS column_default,
This will cause the Go struct to appear as a string. Then all of this reflect stuff can be removed.
There was a problem hiding this comment.
Sweet - didn't know this was there. Updated! Also had to update a few other places that relied on that column being an interface
| @@ -66,6 +66,8 @@ message DatabaseColumn { | |||
| string data_type = 4; | |||
| // The isNullable Flag of the column | |||
| string is_nullable = 5; | |||
| // Indicates where the column has a default value | |||
| bool has_default = 6; | |||
There was a problem hiding this comment.
I would suggest just returning the default value itself here instead of a computed. Since this is the database schema data, we might actually want to use that in the future. Then this can just be computed easily on the frontend.
| bool has_default = 6; | |
| optional string column_default = 6; |
There was a problem hiding this comment.
yeah that can work - I was hesitant at first to do that because our front-end really doesn't touch much "actual" data right now, aka data that would be found in a column but the default value might be okay since I guess it's kinda like metadata? so in the future if we do a hybrid deployment, and we want to isolate any data to the backend/worker, then we would maybe have to reverse this to keep the front-end clean of any data. But maybe it's okay for now.
| !filters.hasDefault && | ||
| transformer.source === TransformerSource.GENERATE_DEFAULT | ||
| ) { | ||
| return true; | ||
| return false; |
There was a problem hiding this comment.
I don't quite understand why this was changed.
We want this block to return true if the filter says that it has a default and the source is Generate default
…return the default
backend/services/mgmt/v1alpha1/connection-data-service/connection-data.go
Outdated
Show resolved
Hide resolved
No description provided.