Skip to content

Commit

Permalink
Merge pull request #456 from CapsicoHealth/455_incorrect_migration_of…
Browse files Browse the repository at this point in the history 
…_strings_when_column_size_changes

#455 incorrect migration logic for strings when sizes change
  • Loading branch information
@ldhasson
ldhasson committed Oct 29, 2019
2 parents b3ccc61 + 68caba5 commit 0866e12
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 94 deletions.
187 changes: 99 additions & 88 deletions src/tilda/db/stores/PostgreSQL.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,22 +99,24 @@ public boolean isLockOrConnectionError(SQLException E)
/**
* Postgres Cancellation codes, from <A href="https://www.postgresql.org/docs/11/errcodes-appendix.html">https://www.postgresql.org/docs/11/errcodes-appendix.html</A>
* <P>
* <UL><LI><B>57000</B>: operator_intervention.</LI>
* <LI><B>57014</B>: query_canceled.</LI>
* <LI><B>57P01</B>: admin_shutdown.</LI>
* <LI><B>57P02</B>: crash_shutdown.</LI>
* <LI><B>57P03</B>: cannot_connect_now.</LI>
* <LI><B>57P04</B>: database_dropped.</LI>
* <UL>
* <LI><B>57000</B>: operator_intervention.</LI>
* <LI><B>57014</B>: query_canceled.</LI>
* <LI><B>57P01</B>: admin_shutdown.</LI>
* <LI><B>57P02</B>: crash_shutdown.</LI>
* <LI><B>57P03</B>: cannot_connect_now.</LI>
* <LI><B>57P04</B>: database_dropped.</LI>
* </UL>
*/
protected static final String[] _CANCEL_SQL_STATES = { "57000", "57014", "57P01", "57P02", "57P03", "57P04" };

protected static final String[] _CANCEL_SQL_STATES = { "57000", "57014", "57P01", "57P02", "57P03", "57P04"
};

@Override
public boolean isCanceledError(SQLException E)
{
return TextUtil.indexOf(E.getSQLState(), _CANCEL_SQL_STATES);
}

@Override
public boolean needsSavepoint()
{
Expand Down Expand Up @@ -383,9 +385,9 @@ public String getColumnType(ColumnType T, Integer S, ColumnMode M, boolean Colle
: "text";
}

return PostgresType.get(T)._SQLType
+ (T == ColumnType.NUMERIC && Precision != null ? "("+ Precision + (Scale != null ? "," + Scale : "") + ")" : "")
+ (T != ColumnType.JSON && Collection == true ? "[]" : "");
return PostgresType.get(T)._SQLType
+ (T == ColumnType.NUMERIC && Precision != null ? "(" + Precision + (Scale != null ? "," + Scale : "") + ")" : "")
+ (T != ColumnType.JSON && Collection == true ? "[]" : "");
}

@Override
Expand Down Expand Up @@ -422,82 +424,85 @@ public boolean alterTableAlterColumnStringSize(Connection Con, ColumnMeta ColMet
// if (ColMetaT == DBStringType.CHARACTER && ColT != DBStringType.CHARACTER)
// Using = " USING rtrim(\"" + Col.getName() + "\")";
String Q = "ALTER TABLE " + Col._ParentObject.getShortName() + " ALTER COLUMN \"" + Col.getName() + "\" TYPE "


+ getColumnType(Col.getType(), Col._Size, Col._Mode, Col.isCollection(), Col._Precision, Col._Scale) + Using + ";";
return Con.executeDDL(Col._ParentObject._ParentSchema._Name, Col._ParentObject.getBaseName(), Q);


}

@Override
public boolean alterTableAlterColumnNumericSize(Connection Con, ColumnMeta ColMeta, Column Col)
throws Exception
{
// // Is precision shrinking?
// if (Col._Precision < ColMeta._Precision)
// {
// String Q = "SELECT length(max(\"" + Col.getName() + "\")::varchar) from " + Col._ParentObject.getShortName();
// ScalarRP RP = new ScalarRP();
// Con.executeSelect(Col._ParentObject._ParentSchema._Name, Col._ParentObject.getBaseName(), Q, RP);
// if (RP.getResult() > Col._Precision+1) //add 1 because varchar conversion includes the decimal in length
// {
// Q = "select \"" + Col.getName() + "\" || ' (' || length(\"" + Col.getName() + "\"::varchar) || ')' as _x from " + Col._ParentObject.getShortName()
// + " group by \"" + Col.getName() + "\""
// + " order by length(\"" + Col.getName() + "\"::varchar) desc"
// + " limit 10";
// StringListRP SLRP = new StringListRP();
// Con.executeSelect(Col._ParentObject._ParentSchema._Name, Col._ParentObject.getBaseName(), Q, SLRP);
// LOG.error("Column sample:");
// for (String s : SLRP.getResult())
// LOG.error(" - " + s);
// throw new Exception("Cannot alter Numeric column '" + Col.getFullName() + "' from precision " + ColMeta._Precision + " down to " + Col._Precision + " because there are values with precision lengths up to " + RP.getResult()
// + " that would cause errors. You need to manually migrate your database.");
// }
// }

// // Is scale expanding?
// if (Col._Scale > ColMeta._Scale)
// {
// String Q = "";
//
// if(ColMeta.isArray() == true)
// Q = "SELECT length(max(\"" + Col.getName() + "\"::int)::varchar) from (select unnest(\"" + Col.getName() + "\") as \"" + Col.getName() + "\" from "+ Col._ParentObject.getShortName() +") as t";
// else
// Q = "SELECT length((max(\"" + Col.getName() + "\")::int)::varchar) from " + Col._ParentObject.getShortName();
// ScalarRP RP = new ScalarRP();
// Con.executeSelect(Col._ParentObject._ParentSchema._Name, Col._ParentObject.getBaseName(), Q, RP);
// long i = RP.getResult();
// if (RP.getResult() > (Col._Precision - Col._Scale)) //digits available for before the decimal
// {
// Q = "select \"" + Col.getName() + "\" || ' (' || length((max(\"" + Col.getName() + "\")::int)::varchar) || ')' as _x from " + Col._ParentObject.getShortName()
// + " group by \"" + Col.getName() + "\""
// + " order by length(\"" + Col.getName() + "\"::varchar) desc"
// + " limit 10";
// StringListRP SLRP = new StringListRP();
// Con.executeSelect(Col._ParentObject._ParentSchema._Name, Col._ParentObject.getBaseName(), Q, SLRP);
// LOG.error("Column sample:");
// for (String s : SLRP.getResult())
// LOG.error(" - " + s);
// throw new Exception("Cannot alter Numeric column '" + Col.getFullName() + "' from scale " + Col._Scale + " up to " + ColMeta._Scale + " because there are pre-decimal value lengths up to " + RP.getResult()
// + " that would cause errors. You need to manually migrate your database.");
// }
// }
// // Is precision shrinking?
// if (Col._Precision < ColMeta._Precision)
// {
// String Q = "SELECT length(max(\"" + Col.getName() + "\")::varchar) from " + Col._ParentObject.getShortName();
// ScalarRP RP = new ScalarRP();
// Con.executeSelect(Col._ParentObject._ParentSchema._Name, Col._ParentObject.getBaseName(), Q, RP);
// if (RP.getResult() > Col._Precision+1) //add 1 because varchar conversion includes the decimal in length
// {
// Q = "select \"" + Col.getName() + "\" || ' (' || length(\"" + Col.getName() + "\"::varchar) || ')' as _x from " + Col._ParentObject.getShortName()
// + " group by \"" + Col.getName() + "\""
// + " order by length(\"" + Col.getName() + "\"::varchar) desc"
// + " limit 10";
// StringListRP SLRP = new StringListRP();
// Con.executeSelect(Col._ParentObject._ParentSchema._Name, Col._ParentObject.getBaseName(), Q, SLRP);
// LOG.error("Column sample:");
// for (String s : SLRP.getResult())
// LOG.error(" - " + s);
// throw new Exception("Cannot alter Numeric column '" + Col.getFullName() + "' from precision " + ColMeta._Precision + " down to " + Col._Precision + " because there are
// values with precision lengths up to " + RP.getResult()
// + " that would cause errors. You need to manually migrate your database.");
// }
// }

// // Is scale expanding?
// if (Col._Scale > ColMeta._Scale)
// {
// String Q = "";
//
// if(ColMeta.isArray() == true)
// Q = "SELECT length(max(\"" + Col.getName() + "\"::int)::varchar) from (select unnest(\"" + Col.getName() + "\") as \"" + Col.getName() + "\" from "+
// Col._ParentObject.getShortName() +") as t";
// else
// Q = "SELECT length((max(\"" + Col.getName() + "\")::int)::varchar) from " + Col._ParentObject.getShortName();
// ScalarRP RP = new ScalarRP();
// Con.executeSelect(Col._ParentObject._ParentSchema._Name, Col._ParentObject.getBaseName(), Q, RP);
// long i = RP.getResult();
// if (RP.getResult() > (Col._Precision - Col._Scale)) //digits available for before the decimal
// {
// Q = "select \"" + Col.getName() + "\" || ' (' || length((max(\"" + Col.getName() + "\")::int)::varchar) || ')' as _x from " + Col._ParentObject.getShortName()
// + " group by \"" + Col.getName() + "\""
// + " order by length(\"" + Col.getName() + "\"::varchar) desc"
// + " limit 10";
// StringListRP SLRP = new StringListRP();
// Con.executeSelect(Col._ParentObject._ParentSchema._Name, Col._ParentObject.getBaseName(), Q, SLRP);
// LOG.error("Column sample:");
// for (String s : SLRP.getResult())
// LOG.error(" - " + s);
// throw new Exception("Cannot alter Numeric column '" + Col.getFullName() + "' from scale " + Col._Scale + " up to " + ColMeta._Scale + " because there are pre-decimal
// value lengths up to " + RP.getResult()
// + " that would cause errors. You need to manually migrate your database.");
// }
// }

String Q = "ALTER TABLE " + Col._ParentObject.getShortName() + " ALTER COLUMN \"" + Col.getName() + "\" TYPE "
+ getColumnType(Col.getType(), Col._Size, Col._Mode, Col.isCollection(), Col._Precision, Col._Scale) + ";";
return Con.executeDDL(Col._ParentObject._ParentSchema._Name, Col._ParentObject.getBaseName(), Q);
}
}


@Override
public boolean alterTableAlterColumnType(Connection Con, ColumnMeta ColMeta, Column Col, ZoneInfo_Data defaultZI)
throws Exception
{

if (ColMeta._TildaType == ColumnType.STRING)
{
if (Col.getType() == ColumnType.DATETIME || Col.getType() == ColumnType.DATE
|| Col.getType() == ColumnType.INTEGER || Col.getType() == ColumnType.LONG || Col.getType() == ColumnType.FLOAT || Col.getType() == ColumnType.DOUBLE
|| Col.getType() == ColumnType.BOOLEAN || Col.getType() == ColumnType.UUID)
|| Col.getType() == ColumnType.BOOLEAN || Col.getType() == ColumnType.UUID)
{
String Q = "ALTER TABLE " + Col._ParentObject.getShortName() + " ALTER COLUMN \"" + Col.getName()
+ "\" TYPE " + getColumnType(Col.getType(), Col._Size, Col._Mode, Col.isCollection(), Col._Precision, Col._Scale)
Expand All @@ -515,12 +520,12 @@ public boolean alterTableAlterColumnType(Connection Con, ColumnMeta ColMeta, Col
else
throw new Exception("Cannot alter a column from " + ColMeta._TildaType + " to " + Col.getType() + ".");
}


String Using = "";
// Looks like we do not need the rtrim call. It slows things down and doesn't actually do anything in Postgres
// if (ColMetaT == DBStringType.CHARACTER && ColT != DBStringType.CHARACTER)
// Using = " USING rtrim(\"" + Col.getName() + "\")";


if (Col.isPrimaryKey() == true || Col.isForeignKey() == true)
{
LOG.warn("\n\n!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\n"
Expand All @@ -539,18 +544,18 @@ public boolean alterTableAlterColumnType(Connection Con, ColumnMeta ColMeta, Col
* Con.ExecuteSelect(Col._ParentObject._ParentSchema._Name, Col._ParentObject.getBaseName(), Q, RP);
*/
String Q = "ALTER TABLE " + Col._ParentObject.getShortName() + " ALTER COLUMN \"" + Col.getName()


+ "\" TYPE " + getColumnType(Col.getType(), Col._Size, Col._Mode, Col.isCollection(), Col._Precision, Col._Scale);
// going to boolean, must use a more elaborate expression to convert
if (Col.getType() == ColumnType.BOOLEAN)
Q += " USING (case when \"" + Col.getName() + "\"=0 then true when \"" + Col.getName() + "\" is null then null else false end)";
else if (ColMeta._TildaType == ColumnType.BOOLEAN)
Q += " USING \"" + Col.getName() + "\"::INTEGER::" + getColumnType(Col.getType(), Col._Size, Col._Mode, Col.isCollection(), Col._Precision, Col._Scale) + ";";
else
else
Q += " USING \"" + Col.getName() + "\"::" + getColumnType(Col.getType(), Col._Size, Col._Mode, Col.isCollection(), Col._Precision, Col._Scale) + ";";


return Con.executeDDL(Col._ParentObject._ParentSchema._Name, Col._ParentObject.getBaseName(), Q);


}

@Override
Expand All @@ -559,13 +564,13 @@ public boolean alterTableAlterColumnMulti(Connection Con, List<ColMetaColPair> B
{
if ((BatchTypeCols == null || BatchTypeCols.size() == 0) && (BatchSizeCols == null || BatchSizeCols.size() == 0))
LOG.error("There are no columns to process for alterTableAlterColumnMulti. Something has gone wrong when adding columns to the AlterColumnMulti migration action.");
String Q = "ALTER TABLE ";
if(BatchTypeCols.size() > 0)

String Q = "ALTER TABLE ";
if (BatchTypeCols.size() > 0)
Q += BatchTypeCols.get(0)._Col._ParentObject.getShortName();
else
Q += BatchSizeCols.get(0)._Col._ParentObject.getShortName();

ArrayList<String> QU = new ArrayList<String>();

// Batch changing ColumnTypes
Expand Down Expand Up @@ -614,10 +619,10 @@ else if (CMP._CMeta._TildaType == ColumnType.BOOLEAN)
Q += " USING \"" + CMP._Col.getName() + "\"::" + getColumnType(CMP._Col.getType(), CMP._Col._Size, CMP._Col._Mode, CMP._Col.isCollection(), CMP._Col._Precision, CMP._Col._Scale) + ",";
}
}
//Batch changing ColumnSize

// Batch changing ColumnSize
for (ColMetaColPair CMP : BatchSizeCols)
{
{
DBStringType ColT = getDBStringType(CMP._Col._Size);
DBStringType ColMetaT = getDBStringType(CMP._CMeta._Size);
// Is it shrinking?
Expand Down Expand Up @@ -648,9 +653,9 @@ else if (CMP._CMeta._TildaType == ColumnType.BOOLEAN)
// if (ColMetaT == DBStringType.CHARACTER && ColT != DBStringType.CHARACTER)
// Using = " USING rtrim(\"" + CMP._Col.getName() + "\")";
Q += "ALTER COLUMN \"" + CMP._Col.getName() + "\" TYPE "
+ getColumnType(CMP._Col.getType(), CMP._Col._Size, CMP._Col._Mode, CMP._Col.isCollection(), CMP._Col._Precision, CMP._Col._Scale) + Using + ",";
+ getColumnType(CMP._Col.getType(), CMP._Col._Size, CMP._Col._Mode, CMP._Col.isCollection(), CMP._Col._Precision, CMP._Col._Scale) + Using + ",";
}

Q = Q.substring(0, Q.length() - 1) + ";";
LOG.info(Q);
if (QU.size() > 0)
Expand All @@ -663,7 +668,13 @@ else if (CMP._CMeta._TildaType == ColumnType.BOOLEAN)
return true;
}
else
return Con.executeDDL(BatchTypeCols.get(0)._Col._ParentObject._ParentSchema._Name, BatchTypeCols.get(0)._Col._ParentObject.getBaseName(), Q);
{
String Schema = BatchTypeCols != null && BatchTypeCols.isEmpty() == false ? BatchTypeCols.get(0)._Col._ParentObject._ParentSchema._Name
: BatchSizeCols.get(0)._Col._ParentObject._ParentSchema._Name;
String Table = BatchTypeCols != null && BatchTypeCols.isEmpty() == false ? BatchTypeCols.get(0)._Col._ParentObject.getBaseName()
: BatchSizeCols.get(0)._Col._ParentObject.getBaseName();
return Con.executeDDL(Schema, Table, Q);
}
}

protected static void PrintFunctionIn(StringBuilder Str, String Type)
Expand Down Expand Up @@ -1013,7 +1024,7 @@ private ColumnType getSubTypeMapping(String Name, String TypeName, ColumnType Ti
{
case "_int2":
TildaType = ColumnType.SHORT;
break;
break;
case "_int4":
TildaType = ColumnType.INTEGER;
break;
Expand All @@ -1028,10 +1039,10 @@ private ColumnType getSubTypeMapping(String Name, String TypeName, ColumnType Ti
break;
case "_numeric":
TildaType = ColumnType.NUMERIC;
break;
break;
case "_uuid":
TildaType = ColumnType.UUID;
break;
break;
case "_bpchar":
TildaType = ColumnType.CHAR;
break;
Expand Down Expand Up @@ -1346,12 +1357,12 @@ public int getMaxTableNameSize()
{
return 63;
}



@Override
public String getBackendConnectionId(Connection connection)
throws Exception
{
return null;
}
}
}
Loading

0 comments on commit 0866e12

Please sign in to comment.