Search tables for TEXT values that are too long for MariaDB #10
Conversation
psql2mysql.py
Outdated
| print(output_table) | ||
| raise Exception("Too long text values found in the source database.") | ||
| except Exception as e: | ||
| print "Error during precheck: ", e |
There was a problem hiding this comment.
please make it python3 compatible print("Error during precheck: ", e)
jsuchome
force-pushed
the
long-chars
branch
from
a5431b2
to
c0a45a8
Compare
psql2mysql.py
Outdated
| @@ -199,7 +230,8 @@ def do_prechecks(config): | |||
| db = DbWrapper(cfg.CONF.source) | |||
| db.connect() | |||
| tables = db.getSortedTables() | |||
| for table in tables: | |||
| try: | |||
There was a problem hiding this comment.
I do not think that is a good idea, as you are hiding the underlining error, like the one from line 255, with a generic 'something is wrong' message, that will require to edit the code to see the real problem when the exception is not an str.
What I mean is that this works only for the exception raised in 255, but there are potentially other Exception that are not string based, so printing e will not provide information.
I suggest simply to fail here, and capture in the caller with more specific cases.
For the line 255, to break the loop I would suggest to simply use break and a print() to show the error, instead of the raise
There was a problem hiding this comment.
"For the line 255, to break the loop I would suggest to simply use break and a print() to show the error, instead of the raise"
OK, that makes sense I guess
psql2mysql.py
Outdated
| @@ -61,6 +61,37 @@ def _exclude_deleted(self, table, query): | |||
| return query.where(table.c.deleted == 'False') | |||
| return query.where(table.c.deleted == False) | |||
|
|
|||
| def getTextColumns(self, table): | |||
| columns = table.columns | |||
| return [ c.name for c in columns if str(c.type) == 'TEXT' ] | |||
There was a problem hiding this comment.
nit: remove the spaces between the brackets?
psql2mysql.py
Outdated
| def scanTableForLongTexts(self, table): | ||
| textColumns = self.getTextColumns(table) | ||
| if not textColumns: | ||
| return [] |
There was a problem hiding this comment.
no need of this as later you iterate over text_columns? eventually your logic will return [], or fails for line 77?
There was a problem hiding this comment.
Well I can skip the query, right?
psql2mysql.py
Outdated
|
|
||
| long_values = [] | ||
| primary_keys = [] | ||
| if not table.primary_key == None: |
There was a problem hiding this comment.
if table.primary_key is not None or if table.primary_key. Both are equivalent.
There was a problem hiding this comment.
Oh, how could I missed this
psql2mysql.py
Outdated
| long_values = [] | ||
| primary_keys = [] | ||
| if not table.primary_key == None: | ||
| primary_keys = list(table.primary_key) |
There was a problem hiding this comment.
I guess that table.primary_key is a single element? if so is more clear this: [table.primary_key]
If can be a single element, or an iterator, I think that our code is OK as in any case will return a list.
There was a problem hiding this comment.
No, there could be more
psql2mysql.py
Outdated
| for row in rows: | ||
| for col in textColumns: | ||
| if not isinstance(row[col], six.string_types): | ||
| continue |
There was a problem hiding this comment.
oh can this happens? be a text column but do not contain text?
There was a problem hiding this comment.
It can actually be None, in which case the next call fails. Comes from rhafer@07fcb15
jsuchome
force-pushed
the
long-chars
branch
from
c0a45a8
to
3ece0ef
Compare
jsuchome
force-pushed
the
long-chars
branch
from
3ece0ef
to
0cfed05
Compare
|
Rebased after merging #11 |
psql2mysql.py
Outdated
| ======= | ||
| MAX_TEXT_LEN = 65536 | ||
|
|
||
| >>>>>>> de99a7b... Search tables for TEXT values that are too long for MariaDB |
There was a problem hiding this comment.
yep, seen that, fixed already
jsuchome
force-pushed
the
long-chars
branch
from
0cfed05
to
5db16d3
Compare
Tracebak does not seem to be necessary if the error message is meaningful.
jsuchome
force-pushed
the
long-chars
branch
from
5db16d3
to
1915010
Compare
2nd commit is not exactly related, and I do not know if you like it this way...