propelorm / Propel2

Propel2 is an open-source high-performance Object-Relational Mapping (ORM) for modern PHP
http://propelorm.org/
MIT License
1.26k stars 393 forks source link

Using addJoin causes where clauses to fail #1919

Open KelseySheely opened 1 year ago

KelseySheely commented 1 year ago

When using addJoin to join a table on column with no foreign key, where clauses are unable to extract the join table name appropriately resulting in the following error:

PHP Fatal error: Uncaught Propel\Runtime\Exception\PropelException: Cannot determine the column to bind to the parameter in clause "Author.First LIKE ?".

Example code is:

$query = BookQuery::create(); $query->addJoin(BookTableMap::COL_AUTHOR_ID, AuthorTableMap::COL_ID, Criteria::INNER_JOIN); $query->where("Author.First LIKE ?", '%Charles%');

It seems that this error comes up because ModelCriteria::getColumnFromName is unable to find the join object because the join is not set up with a key. I am able to get around it by setting up the join object explicitly as a ModelJoin but it should be able to work using the simple addJoin.

Workaround:

$join = new ModelJoin(); $join->setJoinType(Criteria::INNER_JOIN); $join->setTableMap(AuthorTableMap::getTableMap()); $join->setLeftTableName('Book'); $join->setRightTableName('Author'); $join->addCondition(BookTableMap::COL_AUTHOR_ID, AuthorTableMap::COL_ID);

$query = BookQuery::create(); $query->addJoinObject($join, 'Author'); $query->where("Author.First LIKE ?", "%Charles%");

I would suggest either setting the join key in the addJoin method with the value of the right table name or alias and/or adding a block to the ModelCriteria::getColumnFromName which correctly selects the join from the array of joins (similar to how getModelJoinByTableName but not limiting it to ModelJoin and selecting the join based on Alias).

mringler commented 1 year ago

Oh, addJoin() is a low-level method, it is usually better not to use it.

I think the preferred way to do that join or filter through another table is:

BookQuery::create()
    ->useAuthorQuery()
        ->filterByFirstName('%Charles%', Criteria::LIKE)
    ->endUse()
    ->joinWithAuthor() // populates the author field in the returned books, omit it if you just want to filter
    ->find();

If for some reason you absolutely want to use addJoin(), probably the easiest way would be to avoid using the prepared statement variable:

$query = BookQuery::create();
$query->addJoin(BookTableMap::COL_AUTHOR_ID, AuthorTableMap::COL_ID, Criteria::INNER_JOIN);
$query->where("Author.First LIKE '%Charles%'");
KelseySheely commented 1 year ago

I can't use the first way because there is no relation set up (no foreign key). My example may not be a good example, but just assume that there is no relation and I am trying to build a join on any arbitrary column. Maybe I want to find all books whose main character's last name is the same as the author's last name...

It seems dangerous to add a where without using a prepared statement. Would it still be protected against sql injections? I wouldn't really feel comfortable with that without digging in a little more. My workaround is preferable.

KelseySheely commented 1 year ago

For further clarity, I'm actually running a query on an Archive table using the archivable behavior. The archivable behavior removes all foreign keys and therefore relationships on the Archive objects. Therefore I can't use the use functionality and the only way I can search for an archived object by a related archived object is by using the addJoin or addJoinObject method. Imo, even though this is a "low-level" method, there should be an easy way to create an arbitrary join on a query.

mringler commented 1 year ago

Ah, I see. Does something like this work:

$authorQuery = AuthorQuery::create()->filterByFirstName('Neal%', Criteria::LIKE);
BookQuery::create()->whereExists($authorQuery)->find()

Created query looks like this:

SELECT *  
FROM book 
WHERE EXISTS (
  SELECT 1
  FROM author 
  WHERE author.first_name LIKE ?
)

If you need to use a column from the outer query in the exists query, you can add it in a where:

$authorQuery = AuthorQuery::create()->where('Book.Title LIKE CONCAT(\'%\', Author.FirstName,\'%\')');
BookQuery::create()->whereExists($authorQuery)->find()

there should be an easy way to create an arbitrary join on a query

Yes, same with IN queries, support for that is really bad.

KelseySheely commented 1 year ago

That's an interesting solution and might work. Why would it be preferable to the workaround that I posted above?

mringler commented 1 year ago

Oh, your workaround is fine, but it is a lot of work (kudos!) and code to maintain. I think you basically set up the join as Propel does for registered relations. Personally, I would like to have all those internal and low-level structures inaccessible outside Propel, just for how painful it is to work with them. And it should be easier to use Propel than to write the SQL directly.

mringler commented 1 year ago

Uff, just realized the query above obviously does not work, this

SELECT *  
FROM book 
WHERE EXISTS (
  SELECT 1
  FROM author 
  WHERE author.first_name LIKE ?
)

will give you all books, you always need to bind an EXISTS query back to the outer table:

$authorQuery = AuthorQuery::create()
  ->filterByFirstName('Neal%', Criteria::LIKE)
  ->where('Author.Id = Book.AuthorId');
BookQuery::create()->whereExists($authorQuery)->find();

sorry, tired, lol