contain two fix and a feature #97
Conversation
sillydong
commented
Apr 20, 2018
sillydong
commented
- replace \n to space in postgresql comments
- check if schemaname exists to avoid panic
- add step support
Fix: error calling makeTable: failed to render from pipe delimited bytes: record on line 4: wrong number of fields
The old way may cause panic when the retrieved table name don’t contain schema name.
for convenience
environ/funcs.go
Outdated
| @@ -138,6 +139,15 @@ func numbers(start, end int) data.Strings { | |||
| return s | |||
| } | |||
|
|
|||
| // steps returns a live of strings of the numbers start with size of len | |||
There was a problem hiding this comment.
What's a "live" of strings? I don't really understand this comment at all and I can't infer from the code what it's supposed to say.
There was a problem hiding this comment.
That was a misspell. steps act like numbers return a string from start with the length of len.
| @@ -535,7 +537,8 @@ func queryColumnComments(log *log.Logger, db *sql.DB, schemaNames []string) ([]c | |||
| } | |||
|
|
|||
| if c.Valid { | |||
| r.Comment = c.String | |||
| replaced := strings.Replace(c.String, "\n", " ", -1) | |||
There was a problem hiding this comment.
What's the rationale for this change? Might a newline not be a significant part of the comment?
There was a problem hiding this comment.
If there is \n in comment in pg, gnorm will throw error:
error calling makeTable: failed to render from pipe delimited bytes: record on line 4: wrong number of fields
This is about to fix it.
There was a problem hiding this comment.
I just ran into this issue with some of the comments in my database. Can this be merged?
There was a problem hiding this comment.
Then again, this might not be the right fix. I believe I ran into this issue when running gnorm preview on a comment containing | and ). Is there an existing issue ticket for this?
There was a problem hiding this comment.
This pr was about to fix \n in comment. I didn't meet the | or ) issue before.
There was a problem hiding this comment.
For | and ), may be needs more strings.Replace
|
Any update about this issue? Do I need to do modifies? |
