support filtering table definitions #39
Conversation
Codecov ReportBase: 100.00% // Head: 100.00% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #39 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 11 11
Lines 614 625 +11
=========================================
+ Hits 614 625 +11
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
builder/table.go
Outdated
| result := []rel.TableDefinition{} | ||
|
|
||
| for _, def := range table.Definitions { | ||
| if t.DefinitionFilter == nil || t.DefinitionFilter(table, def) { |
There was a problem hiding this comment.
better to move t.DefinitionFilter == nil as early return before calling this loop
if t.DefinitionFilter == nil {
return table.Definitions
}
| table: rel.Table{ | ||
| Op: rel.SchemaAlter, | ||
| Name: "columns", | ||
| Definitions: []rel.TableDefinition{ |
There was a problem hiding this comment.
should we just filter out silently like this? or do you think it's better to print some log? 🤔
There was a problem hiding this comment.
It would be better to have some way to inform users.
There was a problem hiding this comment.
It may be sufficient to output logs in the DefinitionFilter like this
There was a problem hiding this comment.
agree, something like that should be sufficient
There was a problem hiding this comment.
Hi @youpy, could you add the log as well?
There was a problem hiding this comment.
I thought the log will be inside definitions functions?
There was a problem hiding this comment.
There was a problem hiding this comment.
Oh, I misunderstood
added (53e002a)
|
Thank you for this PR |
Yes, but I haven't written any code yet |
builder/table.go
Outdated
| result = append(result, def) | ||
| } else { | ||
| log.Print("[REL] An unsupported table definition has been excluded") | ||
| log.Print("[REL] An unsupported table definition has been excluded: " + reason) |
There was a problem hiding this comment.
won't it be simpler if we just use: log.Printf("[REL] ....: %T", def)?
please use sql@master |
Added a mechanism to exclude unsupported table definitions, example for use with sqlite3 adapters that do not support
ADD CONSTRAINTwhen changing the table schema.