# HG changeset patch # User Asier Lostalé # Date 1437548939 -7200 # Wed Jul 22 09:08:59 2015 +0200 # Node ID 0249dec4ef5cc34fe767d90cf1043e68ef048b30 # Parent d5d21a543e48b1f3302c207b4d6c172755349245 related to bug 30397: DB inconsistencies in PG9.5 Fixed changes in check constraints. OR expressions were grouped differently in pg9.5 than in previous versions. Fixed by getting check constraint definition with pg_get_constraintdef instead of directly from pg_constraint.consrc. After this change, some of the modifications performed in PostgreSqlCheckTranslation are no longer required. diff -r d5d21a543e48 -r 0249dec4ef5c src/org/apache/ddlutils/platform/postgresql/PostgreSqlCheckTranslation.java --- a/src/org/apache/ddlutils/platform/postgresql/PostgreSqlCheckTranslation.java Wed May 27 14:58:33 2015 +0200 +++ b/src/org/apache/ddlutils/platform/postgresql/PostgreSqlCheckTranslation.java Wed Jul 22 09:08:59 2015 +0200 @@ -51,19 +51,11 @@ append(new ReplacePatTranslation("\\[", "")); // handles numeric casting - append(new ReplacePatTranslation("\\(([0-9\\.\\-]+?)\\)::[Nn][Uu][Mm][Ee][Rr][Ii][Cc]", "$1")); - - // remove 1st double brackets if present - // "((type) in (('M', 'P', 'T')))" -> "(type in (('M', 'P', 'T')))" - append(new ReplacePatTranslation("\\((\\(.+)\\)( in .*)", "$1$2")); - - // remove 2nd double brackets if present: - // "(type in (('M', 'P', 'T')))" -> "(type IN ('M', 'P', 'T'))" - append(new ReplacePatTranslation("(.*) in \\((\\(.+\\))\\)", "$1 IN $2")); + append(new ReplacePatTranslation("([0-9\\.\\-]+?)::[Nn][Uu][Mm][Ee][Rr][Ii][Cc]", "$1")); append(new Translation() { public String exec(String s) { - return s.substring(1, s.length() - 1).toUpperCase(); + return s.toUpperCase(); } }); } diff -r d5d21a543e48 -r 0249dec4ef5c src/org/apache/ddlutils/platform/postgresql/PostgreSqlModelLoader.java --- a/src/org/apache/ddlutils/platform/postgresql/PostgreSqlModelLoader.java Wed May 27 14:58:33 2015 +0200 +++ b/src/org/apache/ddlutils/platform/postgresql/PostgreSqlModelLoader.java Wed Jul 22 09:08:59 2015 +0200 @@ -236,7 +236,7 @@ + " WHERE pg_constraint.conrelid = pg_class.oid AND pg_attribute.attrelid = pg_constraint.conrelid AND (pg_attribute.attnum = ANY (pg_constraint.conkey))" + " AND pg_constraint.conname = ?" + " ORDER BY pg_attribute.attnum::integer"); - sql = "SELECT upper(pg_constraint.conname::text), pg_constraint.consrc" + sql = "SELECT upper(pg_constraint.conname::text), regexp_replace(pg_get_constraintdef(pg_constraint.oid, true), E'CHECK \\\\((.*)\\\\).*', E'\\\\1')" + " FROM pg_constraint JOIN pg_class ON pg_class.oid = pg_constraint.conrelid" + " WHERE pg_constraint.contype = 'c' and pg_class.relname = ?"; _stmt_listchecks = _connection.prepareStatement(sql # HG changeset patch # User Asier Lostalé # Date 1437549176 -7200 # Wed Jul 22 09:12:56 2015 +0200 # Node ID b4e3d71c022ad904329f79060ecaf5477c858902 # Parent 0249dec4ef5cc34fe767d90cf1043e68ef048b30 related to bug 30397: DB inconsistencies in PG9.5 Fixed changes of numeric constants in views. When the view contains a numeric constant, in previous versions the query was internally translated to: ... WHEN sd.m_locator_id IS NULL THEN (-1)::numeric * ol.qtyordered ... and in 9.5 is: ... WHEN sd.m_locator_id IS NULL THEN '-1'::integer::numeric * ol.qtyordered ... This case has been taken into account in PostgreSQLStandarization to standarize in both cases to the same SQL. diff -r 0249dec4ef5c -r b4e3d71c022a src/org/apache/ddlutils/platform/postgresql/PostgreSQLStandarization.java --- a/src/org/apache/ddlutils/platform/postgresql/PostgreSQLStandarization.java Wed Jul 22 09:08:59 2015 +0200 +++ b/src/org/apache/ddlutils/platform/postgresql/PostgreSQLStandarization.java Wed Jul 22 09:12:56 2015 +0200 @@ -25,6 +25,13 @@ /** Creates a new instance of PostgreSQLTranslation */ public PostgreSQLStandarization() { + // Starting from 9.5, what before was: + // i.grandtotal * (-1)::numeric + // now is: + // i.grandtotal * '-1'::integer::numeric + // keeping old format + append(new ReplacePatTranslation("'([0-9-]*)'::integer", "\\($1\\)")); + // postgres castings '::text', '::numeric', '::character varying', // '::date', '::bpchar', '::timestamp', '::\"unknown\"' , ::timestamp // with time zone @@ -71,5 +78,4 @@ // ReplacePatTranslation("^[\\s]*(.*?)[\\s]*","$1"))); append(new ReplaceStrTranslation("~~", "LIKE")); } - } # HG changeset patch # User Asier Lostalé # Date 1437549358 -7200 # Wed Jul 22 09:15:58 2015 +0200 # Node ID 8c949bb2abb666b69841994c58d2812d2571ab08 # Parent b4e3d71c022ad904329f79060ecaf5477c858902 related to bug 30397: DB inconsistencies in PG9.5 Fixed brackets in views for some GROUP BY fields. Starting from PostgreSQL 9.5, some fields in GROUP BY are enclosed between brackets. These brackets are removed to keep the same formatting as in previous versions. Example of a modification before this changeset: - + diff -r b4e3d71c022a -r 8c949bb2abb6 src/org/apache/ddlutils/platform/postgresql/PostgreSQLStandarization.java --- a/src/org/apache/ddlutils/platform/postgresql/PostgreSQLStandarization.java Wed Jul 22 09:12:56 2015 +0200 +++ b/src/org/apache/ddlutils/platform/postgresql/PostgreSQLStandarization.java Wed Jul 22 09:15:58 2015 +0200 @@ -16,6 +16,7 @@ import org.apache.ddlutils.translation.CombinedTranslation; import org.apache.ddlutils.translation.ReplacePatTranslation; import org.apache.ddlutils.translation.ReplaceStrTranslation; +import org.apache.ddlutils.translation.Translation; /** * @@ -32,6 +33,40 @@ // keeping old format append(new ReplacePatTranslation("'([0-9-]*)'::integer", "\\($1\\)")); + // Starting from 9.5, some elements in group by are enclosed by brackets, they were not before, + // so let's keep old format + append(new Translation() { + @Override + public String exec(String s) { + if (s.indexOf("GROUP BY") == -1) { + return s; + } + String result = ""; + for (String line : s.split("\n")) { + String modifiedLine; + if (line.trim().startsWith("GROUP BY")) { + modifiedLine = ""; + for (String groupByElement : line.split(",")) { + String modifiedGroupByElement = groupByElement.trim(); + if (modifiedGroupByElement.startsWith("(") && modifiedGroupByElement.endsWith(")")) { + modifiedGroupByElement = modifiedGroupByElement.substring(1, + modifiedGroupByElement.length() - 1); + } + modifiedLine += modifiedGroupByElement + ", "; + } + if (modifiedLine.endsWith(", ") && !line.trim().endsWith(",")) { + modifiedLine = modifiedLine.substring(0, modifiedLine.length() - 2); + } + + } else { + modifiedLine = line; + } + result += modifiedLine + "\n"; + } + return result; + } + }); + // postgres castings '::text', '::numeric', '::character varying', // '::date', '::bpchar', '::timestamp', '::\"unknown\"' , ::timestamp // with time zone # HG changeset patch # User Asier Lostalé # Date 1437550322 -7200 # Wed Jul 22 09:32:02 2015 +0200 # Node ID 4ee14591146b23086862de2fa4ee626a66f06fce # Parent 8c949bb2abb666b69841994c58d2812d2571ab08 related to bug 30397, related to bug 28684: DB inconsistencies in PG9.5 Ignoring test cases created for issue #28684 because they do not apply anymore. Now code for checks are read differently so the modifications previously tested don't apply anymore. diff -r 8c949bb2abb6 -r 4ee14591146b src-test/src/org/openbravo/dbsm/test/model/CheckConstraints.java --- a/src-test/src/org/openbravo/dbsm/test/model/CheckConstraints.java Wed Jul 22 09:15:58 2015 +0200 +++ b/src-test/src/org/openbravo/dbsm/test/model/CheckConstraints.java Wed Jul 22 09:32:02 2015 +0200 @@ -20,7 +20,9 @@ import java.util.List; import org.apache.ddlutils.platform.postgresql.PostgreSqlCheckTranslation; +import org.apache.ddlutils.translation.CombinedTranslation; import org.apache.ddlutils.translation.Translation; +import org.junit.Ignore; import org.junit.Test; /** @@ -29,6 +31,9 @@ * @author alostale * */ +@Ignore("These tests are not applicable anymore after fix for issue #30397 " + + "because constraints are read from db already beautified so extra required modifications " + + "are much more limited.") public class CheckConstraints { private static List contrains; @@ -99,9 +104,13 @@ } } - private String translate(String checkConstraint) throws NoSuchFieldException, + public static String translate(String checkConstraint) throws NoSuchFieldException, IllegalAccessException { - PostgreSqlCheckTranslation trl = new PostgreSqlCheckTranslation(); + return translate(checkConstraint, new PostgreSqlCheckTranslation()); + } + + public static String translate(String checkConstraint, CombinedTranslation trl) + throws NoSuchFieldException, IllegalAccessException { Field f = trl.getClass().getSuperclass().getDeclaredField("_translations"); f.setAccessible(true); @SuppressWarnings("unchecked") # HG changeset patch # User Asier Lostalé # Date 1437550417 -7200 # Wed Jul 22 09:33:37 2015 +0200 # Node ID d7f7a9f54fe7d19fab199606b7260daf0c6b5d42 # Parent 4ee14591146b23086862de2fa4ee626a66f06fce related to bug 30397: DB inconsistencies in PG9.5 Added test cases covering PostgreSQL standardization inconsistencies diff -r 4ee14591146b -r d7f7a9f54fe7 src-test/src/org/openbravo/dbsm/test/model/ModelSuite.java --- a/src-test/src/org/openbravo/dbsm/test/model/ModelSuite.java Wed Jul 22 09:32:02 2015 +0200 +++ b/src-test/src/org/openbravo/dbsm/test/model/ModelSuite.java Wed Jul 22 09:33:37 2015 +0200 @@ -27,6 +27,7 @@ @RunWith(Suite.class) @SuiteClasses({ // CheckConstraints.class, // + Pg95SqlStandardization.class, // Sequences.class, // NumericScaleChanges.class, // AddDropColumn.class, // diff -r 4ee14591146b -r d7f7a9f54fe7 src-test/src/org/openbravo/dbsm/test/model/Pg95SqlStandardization.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src-test/src/org/openbravo/dbsm/test/model/Pg95SqlStandardization.java Wed Jul 22 09:33:37 2015 +0200 @@ -0,0 +1,245 @@ +/* + ************************************************************************************ + * Copyright (C) 2015 Openbravo S.L.U. + * Licensed under the Apache Software License version 2.0 + * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed + * under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR + * CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + ************************************************************************************ + */ + +package org.openbravo.dbsm.test.model; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.junit.Assume.assumeThat; + +import java.util.Arrays; +import java.util.Collection; + +import org.apache.ddlutils.platform.postgresql.PostgreSQLStandarization; +import org.apache.ddlutils.platform.postgresql.PostgreSqlCheckTranslation; +import org.apache.ddlutils.platform.postgresql.PostgreSqlModelLoader; +import org.apache.ddlutils.translation.CombinedTranslation; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +/** + * Test cases covering SQL standardization problems appeared in PostgreSQL 9.5. + * + * See issue #30397 + * + * @author alostale + * + */ +@RunWith(Parameterized.class) +public class Pg95SqlStandardization { + + /** how the contrains sould like once exported to xml file */ + private String expectedTranslation; + + /** + * code in pg 9.4 obtained from + * {@code + * SELECT upper(pg_constraint.conname::text), pg_constraint.consrc + * FROM pg_constraint JOIN pg_class ON pg_class.oid = pg_constraint.conrelid + * WHERE pg_constraint.contype = 'c' + * and pg_constraint.conname ilike ?} + * + * @see PostgreSqlModelLoader + */ + private String pg94Code; + + /** code in pg 9.5 */ + private String pg95Code; + + private CombinedTranslation trl; + + private boolean execute; + + public Pg95SqlStandardization(String constraintName, String expectedTranslation, String pg94Code, + String pg95Code, CombinedTranslation trl, boolean execute) { + this.expectedTranslation = expectedTranslation; + this.pg94Code = pg94Code; + this.pg95Code = pg95Code; + this.trl = trl; + this.execute = execute; + } + + @Parameters(name = "{0}") + public static Collection params() { + return Arrays + .asList(new Object[][] { + { + // Originally this was an issue, which has been fixed by reading constraints from DB + // already beautified so these transformation won't occur anymore -> skipping test + // case + "AD_ORGTYPE_ISLEGALENTITY_CHK", + "(((ISLEGALENTITY = 'Y') AND (ISBUSINESSUNIT = 'N')) OR ((ISLEGALENTITY = 'N') AND (ISBUSINESSUNIT = 'Y'))) OR ((ISLEGALENTITY = 'N') AND (ISBUSINESSUNIT = 'N'))", + "((((islegalentity = 'Y'::bpchar) AND (isbusinessunit = 'N'::bpchar)) OR ((islegalentity = 'N'::bpchar) AND (isbusinessunit = 'Y'::bpchar))) OR ((islegalentity = 'N'::bpchar) AND (isbusinessunit = 'N'::bpchar)))", + "(((islegalentity = 'Y'::bpchar) AND (isbusinessunit = 'N'::bpchar)) OR ((islegalentity = 'N'::bpchar) AND (isbusinessunit = 'Y'::bpchar)) OR ((islegalentity = 'N'::bpchar) AND (isbusinessunit = 'N'::bpchar)))", + new PostgreSqlCheckTranslation(), // + false // + }, + { + // Originally this was an issue, which has been fixed by reading constraints from DB + // already beautified so these transformation won't occur anymore -> skipping test + // case + "C_DEBT_PAYMENT_C_SETTLEMEN_CH1", + "((C_SETTLEMENT_CANCEL_ID IS NOT NULL) OR (C_SETTLEMENT_GENERATE_ID IS NOT NULL)) OR (ISPAID = 'N')", + "(((c_settlement_cancel_id IS NOT NULL) OR (c_settlement_generate_id IS NOT NULL)) OR (ispaid = 'N'::bpchar))", + "((c_settlement_cancel_id IS NOT NULL) OR (c_settlement_generate_id IS NOT NULL) OR (ispaid = 'N'::bpchar))", + new PostgreSqlCheckTranslation(),// + false // + }, + { + "C_INVOICE_V", + "SELECT i.c_invoice_id, i.ad_client_id, i.ad_org_id, i.isactive, i.created, i.createdby, i.updated, i.updatedby, i.issotrx, i.documentno, i.docstatus, i.docaction, i.processing, i.processed, i.c_doctype_id, i.c_doctypetarget_id, i.c_order_id, i.description, i.salesrep_id, i.dateinvoiced, i.dateprinted, i.dateacct, i.c_bpartner_id, i.c_bpartner_location_id, i.ad_user_id, i.poreference, i.dateordered, i.c_currency_id, i.paymentrule, i.c_paymentterm_id, i.c_charge_id, i.m_pricelist_id, i.c_campaign_id, i.c_project_id, i.c_activity_id, i.isprinted, i.isdiscountprinted, CASE WHEN substr(d.docbasetype, 3) = 'C' THEN i.chargeamt * (-1) ELSE i.chargeamt END AS chargeamt, CASE WHEN substr(d.docbasetype, 3) = 'C' THEN i.totallines * (-1) ELSE i.totallines END AS totallines, CASE WHEN substr(d.docbasetype, 3) = 'C' THEN i.grandtotal * (-1) ELSE i.grandtotal END AS grandtotal, CASE WHEN substr(d.docbasetype, 3) = 'C' THEN (-1) ELSE 1 END AS multiplier, CASE WHEN substr(d.docbasetype, 2, 1) = 'P' THEN (-1) ELSE 1 END AS multiplierap, d.docbasetype FROM c_invoice i JOIN c_doctype d ON i.c_doctype_id = d.c_doctype_id ", + "SELECT i.c_invoice_id,\n" + + " i.ad_client_id,\n" + + " i.ad_org_id,\n" + + " i.isactive,\n" + + " i.created,\n" + + " i.createdby,\n" + + " i.updated,\n" + + " i.updatedby,\n" + + " i.issotrx,\n" + + " i.documentno,\n" + + " i.docstatus,\n" + + " i.docaction,\n" + + " i.processing,\n" + + " i.processed,\n" + + " i.c_doctype_id,\n" + + " i.c_doctypetarget_id,\n" + + " i.c_order_id,\n" + + " i.description,\n" + + " i.salesrep_id,\n" + + " i.dateinvoiced,\n" + + " i.dateprinted,\n" + + " i.dateacct,\n" + + " i.c_bpartner_id,\n" + + " i.c_bpartner_location_id,\n" + + " i.ad_user_id,\n" + + " i.poreference,\n" + + " i.dateordered,\n" + + " i.c_currency_id,\n" + + " i.paymentrule,\n" + + " i.c_paymentterm_id,\n" + + " i.c_charge_id,\n" + + " i.m_pricelist_id,\n" + + " i.c_campaign_id,\n" + + " i.c_project_id,\n" + + " i.c_activity_id,\n" + + " i.isprinted,\n" + + " i.isdiscountprinted,\n" + + " CASE\n" + + " WHEN substr(d.docbasetype::text, 3) = 'C'::text THEN i.chargeamt * (-1)::numeric\n" + + " ELSE i.chargeamt\n" + + " END AS chargeamt,\n" + + " CASE\n" + + " WHEN substr(d.docbasetype::text, 3) = 'C'::text THEN i.totallines * (-1)::numeric\n" + + " ELSE i.totallines\n" + + " END AS totallines,\n" + + " CASE\n" + + " WHEN substr(d.docbasetype::text, 3) = 'C'::text THEN i.grandtotal * (-1)::numeric\n" + + " ELSE i.grandtotal\n" + " END AS grandtotal,\n" + + " CASE\n" + + " WHEN substr(d.docbasetype::text, 3) = 'C'::text THEN (-1)\n" + + " ELSE 1\n" + " END AS multiplier,\n" + " CASE\n" + + " WHEN substr(d.docbasetype::text, 2, 1) = 'P'::text THEN (-1)\n" + + " ELSE 1\n" + " END AS multiplierap,\n" + + " d.docbasetype\n" + " FROM c_invoice i\n" + + " JOIN c_doctype d ON i.c_doctype_id::text = d.c_doctype_id::text;", + " SELECT i.c_invoice_id,\n" + + " i.ad_client_id,\n" + + " i.ad_org_id,\n" + + " i.isactive,\n" + + " i.created,\n" + + " i.createdby,\n" + + " i.updated,\n" + + " i.updatedby,\n" + + " i.issotrx,\n" + + " i.documentno,\n" + + " i.docstatus,\n" + + " i.docaction,\n" + + " i.processing,\n" + + " i.processed,\n" + + " i.c_doctype_id,\n" + + " i.c_doctypetarget_id,\n" + + " i.c_order_id,\n" + + " i.description,\n" + + " i.salesrep_id,\n" + + " i.dateinvoiced,\n" + + " i.dateprinted,\n" + + " i.dateacct,\n" + + " i.c_bpartner_id,\n" + + " i.c_bpartner_location_id,\n" + + " i.ad_user_id,\n" + + " i.poreference,\n" + + " i.dateordered,\n" + + " i.c_currency_id,\n" + + " i.paymentrule,\n" + + " i.c_paymentterm_id,\n" + + " i.c_charge_id,\n" + + " i.m_pricelist_id,\n" + + " i.c_campaign_id,\n" + + " i.c_project_id,\n" + + " i.c_activity_id,\n" + + " i.isprinted,\n" + + " i.isdiscountprinted,\n" + + " CASE\n" + + " WHEN substr(d.docbasetype::text, 3) = 'C'::text THEN i.chargeamt * '-1'::integer::numeric\n" + + " ELSE i.chargeamt\n" + + " END AS chargeamt,\n" + + " CASE\n" + + " WHEN substr(d.docbasetype::text, 3) = 'C'::text THEN i.totallines * '-1'::integer::numeric\n" + + " ELSE i.totallines\n" + + " END AS totallines,\n" + + " CASE\n" + + " WHEN substr(d.docbasetype::text, 3) = 'C'::text THEN i.grandtotal * '-1'::integer::numeric\n" + + " ELSE i.grandtotal\n" + + " END AS grandtotal,\n" + + " CASE\n" + + " WHEN substr(d.docbasetype::text, 3) = 'C'::text THEN '-1'::integer\n" + + " ELSE 1\n" + + " END AS multiplier,\n" + + " CASE\n" + + " WHEN substr(d.docbasetype::text, 2, 1) = 'P'::text THEN '-1'::integer\n" + + " ELSE 1\n" + " END AS multiplierap,\n" + + " d.docbasetype\n" + " FROM c_invoice i\n" + + " JOIN c_doctype d ON i.c_doctype_id::text = d.c_doctype_id::text;", + new PostgreSQLStandarization(), // + true // + }, + { + "TEST_V", + "SELECT ps.duedate, trunc(now()) - trunc(ps.duedate) AS daysoverdue1, ps.created count(*) AS count FROM fin_payment_schedule ps GROUP BY ps.duedate, trunc(now()) - trunc(ps.duedate), ps.created ", + " SELECT ps.duedate, trunc(now()) - trunc(ps.duedate) AS daysoverdue1, ps.created\n" + + " count(*) AS count\n" // + + " FROM fin_payment_schedule ps\n" + + " GROUP BY ps.duedate, trunc(now()) - trunc(ps.duedate), ps.created;", + " SELECT ps.duedate, trunc(now()) - trunc(ps.duedate) AS daysoverdue1, ps.created\n" + + " count(*) AS count\n" // + + " FROM fin_payment_schedule ps\n" + + " GROUP BY ps.duedate, (trunc(now()) - trunc(ps.duedate)), ps.created;", + new PostgreSQLStandarization(), // + true // + } // + }); + } + + @Test + public void sqlShouldBeTranslatedToSameString() throws NoSuchFieldException, + IllegalAccessException { + assumeThat("Should be executed", execute, is(true)); + System.out.println("----- 9.4 -----"); + assertThat("pg 9.4", CheckConstraints.translate(pg94Code, trl), equalTo(expectedTranslation)); + System.out.println("\n----- 9.5 -----"); + assertThat("pg 9.5", CheckConstraints.translate(pg95Code, trl), equalTo(expectedTranslation)); + } +}