Enable to specify set of conditional expression set in ScanAll

[jnmt]

This PR enables specifying a set of conditional expression sets in ScanAll. The main reason to add this interface is to easily build a where clause without calling builder methods several times (especially when we build ScanAll for an internal purpose, e.g., in ScalarDB SQL).

[jnmt]

This interface is mainly for internal purposes, but users can see the interface and might use it. So, I didn't use Conjunction here.

[Torch3333]

LGTM, thank you!

[brfrn169]

Can you please provide some examples to help us understand when this would be useful? I'm curious as to why the existing methods don't suffice.

[jnmt]

@brfrn169 Thank you for the question. When we have a set of conjunctions, we need to call where() and or() several times and change the behavior depending on the number of conjunctions. I just wanted to avoid managing an intermediate builder like the following.

BuildableScanAll buildableScanAll =
    Scan.newBuilder()
        .namespace(statement.namespaceName)
        .table(statement.tableName)
        .all();

BuildableScanAllWithOngoingWhereOr ongoing = null;
for (Conjunction conjunction : conjunctions) {
  // snip conversion of conjunction in the SQL layer -> conditional expression
  if (ongoing == null) {
     ongoing = builder.where(ConditionSetBuilder.andConditionSet(conditions).build());
  } else {
     ongoing.or(ConditionSetBuilder.andConditionSet(conditions).build());
  }
}

[brfrn169]

@jnmt Thank you! I understand.

I'm not sure this will works, but instead of introducing the new interface WhereConditions<T>, can we add alternative where() methods that accepts a set of AndConditionSet to WhereOr<T> as follows?

  interface WhereOr<T> {
    /**
     * Appends the specified set of and-wise conditions.
     *
     * @param conditions a set of conditions
     * @return the operation builder
     */
    T where(ScanBuilder.AndConditionSet conditions);

    // ADDED
    T where(Set<ScanBuilder.AndConditionSet> conditions);
  }

My concern is that the newly added method T where(Set<Set<ConditionalExpression>> conditions) is somewhat unclear, and users may struggle to predict the function of the method based on its signature. What do you think about this?

[jnmt]

@brfrn169 That's the point that bothered me (since it's somewhat imbalanced in and/or set, how about adding a message like "internal purpose", and so on and so on...). Anyway, after reconsidering, I think your idea is better. Fixed in acd3be2. Thank you!

[komamitsu]

LGTM! 👍

[brfrn169]

BTW, can we also add where(Set<ScanBuilder.OrConditionSet> andConditionSets) to WhereAnd<T> as well for the sake of consistency, it could cause signature conflicts though?

[jnmt]

Unfortunately, we cannot add it since it causes the signature conflict, as you guess.

[brfrn169]

Okay. In that case, how about changing the method name to address the signature conflict as follows?

  interface WhereAnd<T> {
    T where(ScanBuilder.OrConditionSet orConditionSet);

    T whereAnd(Set<ScanBuilder.OrConditionSet> orConditionSets);
  }

  interface WhereOr<T> {
    T where(ScanBuilder.AndConditionSet andConditionSet);

    T whereOr(Set<ScanBuilder.AndConditionSet> andConditionSets);
  }

[jnmt]

Thanks for the solution. Umm, it's a bit unstylish, but right, consistency is more important here, I think.

[jnmt]

Fixed in 1b9ae6a. PTAL when you have time!

[brfrn169]

LGTM! Thank you!

[feeblefakie]

LGTM! Thank you!