Deprecate Join::ON ?

[HypeMC]

I've been wondering about this for years, what is the purpose of the Join::ON constant? This constant seems completely useless, as the DQL parser doesn't even support the ON keyword in a join:

$fooRepository->createQueryBuilder('f')
    ->innerJoin('f.bar', 'b', Join::ON, 'b.condition = 1')
    ->getQuery()
    ->execute()
;

[Syntax Error] line 0, col 51: Error: Expected end of string, got 'ON'

I haven't found any issues related to this, nor any explanations regarding the original idea behind this constant.

Also, if it's decided to deprecate and remove this constant, the $conditionType parameters of the QueryBuilder::*Join() methods and Expr\Join class will become redundant since currently, the only two options are WITH and ON. Therefore, it might be a good idea to consider deprecating them as well.

orm/lib/Doctrine/ORM/QueryBuilder.php

Lines 1028 to 1052 in 23d36c0

	* @param string $alias The alias of the join.
	* @param string|null $conditionType The condition type constant. Either ON or WITH.
	* @param string|Expr\Comparison|Expr\Composite|Expr\Func|null $condition The condition for the join.
	* @param string|null $indexBy The index for the join.
	* @psalm-param Expr\Join::ON|Expr\Join::WITH|null $conditionType
	*
	* @return $this
	*/
	public function innerJoin($join, $alias, $conditionType = null, $condition = null, $indexBy = null)
	{
	$parentAlias = substr($join, 0, (int) strpos($join, '.'));
	$rootAlias = $this->findRootAlias($alias, $parentAlias);
	$join = new Expr\Join(
	Expr\Join::INNER_JOIN,
	$join,
	$alias,
	$conditionType,
	$condition,
	$indexBy
	);
	return $this->add('join', [$rootAlias => $join], true);
	}

[github-actions]

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.
If you want to continue working on it, please leave a comment.

[derrabus]

Sorry that we haven't picked this up yet.

Does that mean that Join::WITH is the only valid value we can pass as the third argument of innerJoin()? If so, does it make sense to maintain a parameter with only a single valid value? Can we also deprecate the Join::WITH constant?

[HypeMC]

Sorry that we haven't picked this up yet.

Does that mean that Join::WITH is the only valid value we can pass as the third argument of innerJoin()? If so, does it make sense to maintain a parameter with only a single valid value? Can we also deprecate the Join::WITH constant?

@derrabus Yes, that's right, Join::WITH is the only valid value. IMO, it would make sense to deprecate the parameter altogether, as it's currently useless.

[github-actions]

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.
If you want to continue working on it, please leave a comment.