Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Paranthesis in Escape Sequence does not work anymore #865

Closed
tkdmatze opened this issue Jul 12, 2017 · 30 comments
Closed

Paranthesis in Escape Sequence does not work anymore #865

tkdmatze opened this issue Jul 12, 2017 · 30 comments
Milestone

Comments

@tkdmatze
Copy link

we use Crystalreports and their QueryBuilder,

the have outer joins like this
{oj (\"T1\" LEFT OUTER JOIN t2 ON \"T1\".id = t2.id) }
they are not translated anymore

may be connected to
00a8478

because QUOTE_OR_ALPHABETIC_MARKER does not allow Paranthesis to be the first letter

was working in version 9.4.1211
in current version its broken

@vlsi
Copy link
Member

vlsi commented Jul 12, 2017

Here's relevant MySQL/J thread: https://bugs.mysql.com/bug.php?id=28317#c146163

The thing is the specification does not allow ( after {oj, so this must be an issue of Crystalreports.

@AlBundy33
Copy link
Contributor

@AlBundy33
Copy link
Contributor

Without this fix an update of a newer postgres-driver is impossible and so we're stuck on postgres 9.5 because of another error (org.postgresql.util.PSQLException: ERROR: column am.amcanorder does not
exist)

@davecramer
Copy link
Member

Well the spec does not allow ( after {oj
Are you suggesting we fix this instead of Crystal reports ?

@AlBundy33
Copy link
Contributor

It would be nice because it worked for several years. :-)
If I find a bug tracker for crystal reports I'll report this there too.

@luetzelbinge
Copy link

I have created the following test:

package test;

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;

public class PgsqlOuterJoinTest
{
/**

  • The name of the host, on whic
    */
    public static final String HOST = "localhost"; // "myHost";
    public static final String PORT = "5432"; // "myPort";
    public static final String DBNAME = "outerjointest"; // "myDb";
    public static final String USER = "postgres";
    public static final String PASSWD = ""; // "myPass";

public static void main(final String[] args)
throws Exception
{
Connection _connection = null;

try
{
  _connection  = createConnection();
  
  // create the test tables and fill them with test data
  initialize(_connection);
  
  // execute the statement
  ResultSet _rs = null;
  Statement _statement = null;
  try
  {
    _statement = _connection.createStatement();  

    try
    {
      // The simple outer join works fine
      // --> Query: select * from {oj mytable m left outer join yourtable y on (m.id=y.id)}
      
      // Here is the test query with the netsted outer join syntax, that failes
      _rs = _statement.executeQuery("SELECT * FROM   {oj (mytable m LEFT OUTER JOIN yourtable y ON m.id=y.id) "
          + "LEFT OUTER JOIN ourtable o ON m.id=o.id}");
      
      System.err.println(_rs.next() ? "SUCCESS" : "FAILURE");
    }
    finally 
    {
      if (_rs != null)
      {
        _rs.close();
      }
    }
  }
  finally 
  {
    if (_statement != null)
    {
      _statement.close();
    }
  }

  
  
}
finally 
{
  if (_connection != null)
  {
    _connection.close();
  }
}

}

private static Connection createConnection ()
throws SQLException, ClassNotFoundException
{
Class.forName("org.postgresql.Driver");

final Connection _connection = DriverManager.getConnection(
   "jdbc:postgresql://" + HOST + ":" + PORT + "/" + DBNAME, USER, PASSWD);

return _connection;

}

private static void initialize(final Connection theConnection)
throws SQLException
{
createTestTables(theConnection);

fillData(
    theConnection, 
    "myTable", 
    new Object[][] {
        {1, "my1"},
        {2, "my2"},
        {3, "my3"},
    });

fillData(
    theConnection, 
    "yourTable", 
    new Object[][] {
        {11, "your11"},
        {12, "your12"},
        {13, "your13"},
    });

fillData(
    theConnection, 
    "ourTable", 
    new Object[][] {
        {21, "our21"},
        {22, "our22"},
        {23, "our23"},
    });

}

private static boolean existsTable (final Connection theConnection, final String theTableName)
throws SQLException
{
PreparedStatement _statement = null;

try
{
  _statement = theConnection.prepareStatement(
      "select tablename from pg_tables where tablename = ?");  

  _statement.setString(1, theTableName);

  ResultSet _rs = null;
  try
  {
    _rs = _statement.executeQuery();
    
    return _rs.next();
  }
  finally 
  {
    if (_rs != null)
    {
      _rs.close();
    }
  }
}
finally 
{
  if (_statement != null)
  {
    _statement.close();
  }
}

}

private static void createTestTables (final Connection theConnection)
throws SQLException
{

Statement _statement = null;

try
{
  _statement = theConnection.createStatement();  

  if (! existsTable(theConnection, "mytable"))
  {
    final String _tableDef1 = "create table mytable (id int not null, text varchar(255) not null)";
    _statement.executeUpdate(_tableDef1);
  }
  
  
  if (! existsTable(theConnection, "yourtable"))
  {
    final String _tableDef2 = "create table yourtable (id int not null, text varchar(255) not null)";
    _statement.executeUpdate(_tableDef2);
  }
  
  if (! existsTable(theConnection, "ourtable"))
  {
    final String _tableDef2 = "create table ourtable (id int not null, text varchar(255) not null)";
    _statement.executeUpdate(_tableDef2);
  }
}
finally 
{
  if (_statement != null)
  {
    _statement.close();
  }
}

}

private static void fillData(
final Connection theConnection,
final String theTableName,
final Object[][] theValues)
throws SQLException
{
PreparedStatement _statement = null;

try
{
  _statement = theConnection.prepareStatement(
      "insert into " + theTableName + " (id, text) values (?,?)");  


  for (final Object[] _currRow : theValues)
  {
    _statement.setInt(1, (Integer)_currRow[0]);
    _statement.setString(2, (String)_currRow[1]);
    
    _statement.executeUpdate();
  }
}
finally 
{
  if (_statement != null)
  {
    _statement.close();
  }
}

}
}

@polobo
Copy link

polobo commented Mar 26, 2018

Well the spec does not allow ( after {oj
Are you suggesting we fix this instead of Crystal reports ?

Unless there is evidence of why us allowing this interferes with the spec I would say our historical allowance (and probably conformance to other drivers) trumps strict standard conformance concerns.

IOW, I don't see the harm in accepting it - then again I haven't tried to code the fix given other subsequent changes...

@davecramer
Copy link
Member

@polobo I don't disagree, although this really belongs in Crystal Reports bug tracker

@davecramer
Copy link
Member

@luetzelbinge Thanks! It would be really great if you could create a test that worked in the test suite the driver comes with

@luetzelbinge
Copy link

OuterJoinSyntaxTest.tar.gz

@davecramer I've reworked the test. I used the current snapshot of the postgres driver (42.2.2). Because I don't know where to locate the test in the package hierarchy, I give you the test in the attachment.

@vlsi
Copy link
Member

vlsi commented Mar 27, 2018

@luetzelbinge , here's how one finds relevant package: https://github.com/pgjdbc/pgjdbc/search?l=Java&q=oj&type=&utf8=%E2%9C%93

@vlsi vlsi added this to the 42.2.3 milestone Mar 27, 2018
@luetzelbinge
Copy link

OuterJoinSyntaxTest.tar.gz

Hi again,
I moved the test into the package org.postgresql.test.jdbc2 because the ParserTest is affected by Jdbc2TestSuite . Both classes are part of the upload in this comment.

Best regards
Ron

@AlBundy33
Copy link
Contributor

Adding to a milestone sounds good. :-)
Thanks for that.

@vlsi
Copy link
Member

vlsi commented Mar 27, 2018

@luetzelbinge , could you file the code as a GitHub PR?

@davecramer
Copy link
Member

+1 to PR. This makes it almost trivial to test

@luetzelbinge
Copy link

Added test to the repository as a PR.
#1156

Happy Easter
Ron

@AlBundy33
Copy link
Contributor

Any news about this issue?

@davecramer
Copy link
Member

the pr #1156 needs work. Care to take a run at it ?

@AlBundy33
Copy link
Contributor

I've checked #1156 and created a new pr #1204 with single and multiple joins, with and without oj-syntax.

With driver version 9.4.12.11 all tests a green with master all tests with "WithOj" are red.

@AlBundy33
Copy link
Contributor

Is this test from my pr now ok?

@vlsi
Copy link
Member

vlsi commented May 29, 2018

@AlBundy33

  1. There are style issues: https://travis-ci.org/pgjdbc/pgjdbc/jobs/384675444#L539-L572
  2. There are test issues: https://travis-ci.org/pgjdbc/pgjdbc/jobs/384675450#L1193-L1251

For instance,

org.postgresql.util.PSQLException: ERROR: syntax error at or near "{"
  Position: 15
	at org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2433)

means the driver did sent an invalid SQL to the backend. Apparently that means the PR is not ready yet.

@AlBundy33
Copy link
Contributor

  1. ok - I have not seen this. :-) I'll check this.
  2. thats right - the test is intended to the outer join syntax as it has worked up to version 9.4.1211 ;-)

@AlBundy33
Copy link
Contributor

  1. checkstyle issues are fixed now - I haven't seen that you have a checkstyle-configuration.
  2. this error 'syntax error at or near "{"' are the ones, that should be fixed.

@AlBundy33
Copy link
Contributor

@vlsi have you checked #1204?
In my opinion this should be ok now.

@vlsi
Copy link
Member

vlsi commented May 30, 2018

@AlBundy33 , the build is KO: https://travis-ci.org/pgjdbc/pgjdbc/builds/385204044

Apparently ERROR: syntax error at or near "{" has not yet been resolved

@AlBundy33
Copy link
Contributor

AlBundy33 commented May 30, 2018

Of course not - fixing this is part of the pgjdbc-developers. :-)
As requested by you and @davecramer @luetzelbinge has created a pr with a test for this issue.
According to @davecramer the pr was not accepted because of your comments.

Therefore I've created a new pr #1204 with a modified test according to your comments.

@AlBundy33
Copy link
Contributor

@tkdmatze was absolutely right with his hints.
I've changed the Parser to allow parenthesis after oj - tests are green and crystal reports seems to work too.

@AlBundy33
Copy link
Contributor

@vlsi and @davecramer is there any timeline when the open pull requests will be merged and a new version is released?

@davecramer
Copy link
Member

@AlBundy33 not really. more or less as soon as possible; that being said it is mostly dependant on when we find time to review them

@AlBundy33
Copy link
Contributor

Ok. :-)
Maybe you can remove the pr from @luetzelbinge and add mine to milestone.
https://github.com/pgjdbc/pgjdbc/milestone/17

vlsi pushed a commit that referenced this issue Jun 4, 2018
Non-standard `{oj (...)}` is produced by CrystalReports, so enable that deviation from the spec and ignore the parenthesis.

Note: this basically reverts "strict" part of #657
@vlsi vlsi closed this as completed Jun 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants