From a2327a0cd13d363c04c8944620419c31019001ed Mon Sep 17 00:00:00 2001
From: Mark Paluch <mpaluch@pivotal.io>
Date: Tue, 18 Feb 2020 09:17:27 +0100
Subject: [PATCH 1/2] #289 - Prepare issue branch.

---
 pom.xml | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/pom.xml b/pom.xml
index f0f6bc45..662c76bb 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1,11 +1,13 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
 
 	<modelVersion>4.0.0</modelVersion>
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-r2dbc</artifactId>
-	<version>1.1.0.BUILD-SNAPSHOT</version>
+	<version>1.1.0.gh-289-SNAPSHOT</version>
 
 	<name>Spring Data R2DBC</name>
 	<description>Spring Data module for R2DBC</description>
@@ -22,7 +24,7 @@
 		<dist.key>DATAR2DBC</dist.key>
 
 		<springdata.commons>2.3.0.BUILD-SNAPSHOT</springdata.commons>
-		<springdata.jdbc>2.0.0.BUILD-SNAPSHOT</springdata.jdbc>
+		<springdata.jdbc>2.0.0.DATAJDBC-490-SNAPSHOT</springdata.jdbc>
 		<springdata.relational>${springdata.jdbc}</springdata.relational>
 		<java-module-name>spring.data.r2dbc</java-module-name>
 		<sonar.dynamicAnalysis>reuseReports</sonar.dynamicAnalysis>

From 649371bcc13d9492b3f7499f2eaa6d9508872563 Mon Sep 17 00:00:00 2001
From: Mark Paluch <mpaluch@pivotal.io>
Date: Tue, 18 Feb 2020 09:33:31 +0100
Subject: [PATCH 2/2] #289 - Add support for Criteria composition.

We now support composition of Criteria objects to create a Criteria from one or more top-level criteria and to compose nested AND/OR Criteria objects:

Criteria.where("name").is("Foo")).and(Criteria.where("name").is("Bar").or("age")
				.lessThan(49).or(Criteria.where("name").not("Bar").and("age").greaterThan(49))
---
 .../r2dbc/core/DefaultStatementMapper.java    |   6 +-
 .../data/r2dbc/core/StatementMapper.java      |  18 +-
 .../data/r2dbc/query/Criteria.java            | 213 ++++++++++++++++--
 .../data/r2dbc/query/QueryMapper.java         |  87 ++++++-
 .../data/r2dbc/query/CriteriaUnitTests.java   |  55 +++++
 .../r2dbc/query/QueryMapperUnitTests.java     |  70 ++++++
 6 files changed, 415 insertions(+), 34 deletions(-)

diff --git a/src/main/java/org/springframework/data/r2dbc/core/DefaultStatementMapper.java b/src/main/java/org/springframework/data/r2dbc/core/DefaultStatementMapper.java
index 2ccc8fc7..52a29e05 100644
--- a/src/main/java/org/springframework/data/r2dbc/core/DefaultStatementMapper.java
+++ b/src/main/java/org/springframework/data/r2dbc/core/DefaultStatementMapper.java
@@ -88,7 +88,7 @@ private PreparedOperation<Select> getMappedObject(SelectSpec selectSpec,
 		BindMarkers bindMarkers = this.dialect.getBindMarkersFactory().create();
 		Bindings bindings = Bindings.empty();
 
-		if (selectSpec.getCriteria() != null) {
+		if (!selectSpec.getCriteria().isEmpty()) {
 
 			BoundCondition mappedObject = this.updateMapper.getMappedObject(bindMarkers, selectSpec.getCriteria(), table,
 					entity);
@@ -203,7 +203,7 @@ private PreparedOperation<Update> getMappedObject(UpdateSpec updateSpec,
 
 		Update update;
 
-		if (updateSpec.getCriteria() != null) {
+		if (!updateSpec.getCriteria().isEmpty()) {
 
 			BoundCondition boundCondition = this.updateMapper.getMappedObject(bindMarkers, updateSpec.getCriteria(), table,
 					entity);
@@ -237,7 +237,7 @@ private PreparedOperation<Delete> getMappedObject(DeleteSpec deleteSpec,
 		Bindings bindings = Bindings.empty();
 
 		Delete delete;
-		if (deleteSpec.getCriteria() != null) {
+		if (!deleteSpec.getCriteria().isEmpty()) {
 
 			BoundCondition boundCondition = this.updateMapper.getMappedObject(bindMarkers, deleteSpec.getCriteria(), table,
 					entity);
diff --git a/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java b/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java
index 88495514..a9c29f65 100644
--- a/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java
+++ b/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java
@@ -185,7 +185,7 @@ class SelectSpec {
 		private final Table table;
 		private final List<String> projectedFields;
 		private final List<Expression> selectList;
-		private final @Nullable Criteria criteria;
+		private final Criteria criteria;
 		private final Sort sort;
 		private final long offset;
 		private final int limit;
@@ -219,7 +219,7 @@ public static SelectSpec create(String table) {
 		 * @since 1.1
 		 */
 		public static SelectSpec create(SqlIdentifier table) {
-			return new SelectSpec(Table.create(table), Collections.emptyList(), Collections.emptyList(), null,
+			return new SelectSpec(Table.create(table), Collections.emptyList(), Collections.emptyList(), Criteria.empty(),
 					Sort.unsorted(), -1, -1);
 		}
 
@@ -463,9 +463,9 @@ class UpdateSpec {
 		@Nullable
 		private final Update update;
 
-		private final @Nullable Criteria criteria;
+		private final Criteria criteria;
 
-		protected UpdateSpec(SqlIdentifier table, @Nullable Update update, @Nullable Criteria criteria) {
+		protected UpdateSpec(SqlIdentifier table, @Nullable Update update, Criteria criteria) {
 
 			this.table = table;
 			this.update = update;
@@ -490,7 +490,7 @@ public static UpdateSpec create(String table, Update update) {
 		 * @since 1.1
 		 */
 		public static UpdateSpec create(SqlIdentifier table, Update update) {
-			return new UpdateSpec(table, update, null);
+			return new UpdateSpec(table, update, Criteria.empty());
 		}
 
 		/**
@@ -512,7 +512,6 @@ public Update getUpdate() {
 			return this.update;
 		}
 
-		@Nullable
 		public Criteria getCriteria() {
 			return this.criteria;
 		}
@@ -525,9 +524,9 @@ class DeleteSpec {
 
 		private final SqlIdentifier table;
 
-		private final @Nullable Criteria criteria;
+		private final Criteria criteria;
 
-		protected DeleteSpec(SqlIdentifier table, @Nullable Criteria criteria) {
+		protected DeleteSpec(SqlIdentifier table, Criteria criteria) {
 			this.table = table;
 			this.criteria = criteria;
 		}
@@ -550,7 +549,7 @@ public static DeleteSpec create(String table) {
 		 * @since 1.1
 		 */
 		public static DeleteSpec create(SqlIdentifier table) {
-			return new DeleteSpec(table, null);
+			return new DeleteSpec(table, Criteria.empty());
 		}
 
 		/**
@@ -567,7 +566,6 @@ public SqlIdentifier getTable() {
 			return this.table;
 		}
 
-		@Nullable
 		public Criteria getCriteria() {
 			return this.criteria;
 		}
diff --git a/src/main/java/org/springframework/data/r2dbc/query/Criteria.java b/src/main/java/org/springframework/data/r2dbc/query/Criteria.java
index 858ad83a..4d2d9ee1 100644
--- a/src/main/java/org/springframework/data/r2dbc/query/Criteria.java
+++ b/src/main/java/org/springframework/data/r2dbc/query/Criteria.java
@@ -17,6 +17,8 @@
 
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
 
 import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.relational.core.sql.SqlIdentifier;
@@ -27,33 +29,106 @@
  * Central class for creating queries. It follows a fluent API style so that you can easily chain together multiple
  * criteria. Static import of the {@code Criteria.property(…)} method will improve readability as in
  * {@code where(property(…).is(…)}.
+ * <p>
+ * The Criteria API supports composition with a {@link #empty() NULL object} and a {@link #from(List) static factory
+ * method}. Example usage:
+ *
+ * <pre class="code">
+ * Criteria.from(Criteria.where("name").is("Foo"), Criteria.from(Criteria.where("age").greaterThan(42)));
+ * </pre>
+ *
+ * rendering:
+ *
+ * <pre class="code">
+ * WHERE name = 'Foo' AND age > 42
+ * </pre>
  *
  * @author Mark Paluch
  * @author Oliver Drotbohm
  */
 public class Criteria {
 
+	private static final Criteria EMPTY = new Criteria(SqlIdentifier.EMPTY, Comparator.INITIAL, null);
+
 	private final @Nullable Criteria previous;
 	private final Combinator combinator;
+	private final List<Criteria> group;
 
-	private final SqlIdentifier column;
-	private final Comparator comparator;
+	private final @Nullable SqlIdentifier column;
+	private final @Nullable Comparator comparator;
 	private final @Nullable Object value;
 
 	private Criteria(SqlIdentifier column, Comparator comparator, @Nullable Object value) {
-		this(null, Combinator.INITIAL, column, comparator, value);
+		this(null, Combinator.INITIAL, Collections.emptyList(), column, comparator, value);
 	}
 
-	private Criteria(@Nullable Criteria previous, Combinator combinator, SqlIdentifier column, Comparator comparator,
-			@Nullable Object value) {
+	private Criteria(@Nullable Criteria previous, Combinator combinator, List<Criteria> group,
+			@Nullable SqlIdentifier column, @Nullable Comparator comparator, @Nullable Object value) {
 
 		this.previous = previous;
-		this.combinator = combinator;
+		this.combinator = previous != null && previous.isEmpty() ? Combinator.INITIAL : combinator;
+		this.group = group;
 		this.column = column;
 		this.comparator = comparator;
 		this.value = value;
 	}
 
+	private Criteria(@Nullable Criteria previous, Combinator combinator, List<Criteria> group) {
+
+		this.previous = previous;
+		this.combinator = previous != null && previous.isEmpty() ? Combinator.INITIAL : combinator;
+		this.group = group;
+		this.column = null;
+		this.comparator = null;
+		this.value = null;
+	}
+
+	/**
+	 * Static factory method to create an empty Criteria.
+	 *
+	 * @return an empty {@link Criteria}.
+	 * @since 1.1
+	 */
+	public static Criteria empty() {
+		return EMPTY;
+	}
+
+	/**
+	 * Create a new {@link Criteria} and combine it as group with {@code AND} using the provided {@link List Criterias}.
+	 *
+	 * @return new {@link Criteria}.
+	 * @since 1.1
+	 */
+	public static Criteria from(Criteria... criteria) {
+
+		Assert.notNull(criteria, "Criteria must not be null");
+		Assert.noNullElements(criteria, "Criteria must not contain null elements");
+
+		return from(Arrays.asList(criteria));
+	}
+
+	/**
+	 * Create a new {@link Criteria} and combine it as group with {@code AND} using the provided {@link List Criterias}.
+	 *
+	 * @return new {@link Criteria}.
+	 * @since 1.1
+	 */
+	public static Criteria from(List<Criteria> criteria) {
+
+		Assert.notNull(criteria, "Criteria must not be null");
+		Assert.noNullElements(criteria, "Criteria must not contain null elements");
+
+		if (criteria.isEmpty()) {
+			return EMPTY;
+		}
+
+		if (criteria.size() == 1) {
+			return criteria.get(0);
+		}
+
+		return EMPTY.and(criteria);
+	}
+
 	/**
 	 * Static factory method to create a Criteria using the provided {@code column} name.
 	 *
@@ -77,14 +152,43 @@ public CriteriaStep and(String column) {
 
 		Assert.hasText(column, "Column name must not be null or empty!");
 
-		return new DefaultCriteriaStep(SqlIdentifier.unquoted(column)) {
+		SqlIdentifier identifier = SqlIdentifier.unquoted(column);
+		return new DefaultCriteriaStep(identifier) {
 			@Override
 			protected Criteria createCriteria(Comparator comparator, Object value) {
-				return new Criteria(Criteria.this, Combinator.AND, SqlIdentifier.unquoted(column), comparator, value);
+				return new Criteria(Criteria.this, Combinator.AND, Collections.emptyList(), identifier, comparator, value);
 			}
 		};
 	}
 
+	/**
+	 * Create a new {@link Criteria} and combine it as group with {@code AND} using the provided {@link Criteria} group.
+	 *
+	 * @param criteria criteria object.
+	 * @return a new {@link Criteria} object.
+	 * @since 1.1
+	 */
+	public Criteria and(Criteria criteria) {
+
+		Assert.notNull(criteria, "Criteria must not be null!");
+
+		return and(Collections.singletonList(criteria));
+	}
+
+	/**
+	 * Create a new {@link Criteria} and combine it as group with {@code AND} using the provided {@link Criteria} group.
+	 *
+	 * @param criteria criteria objects.
+	 * @return a new {@link Criteria} object.
+	 * @since 1.1
+	 */
+	public Criteria and(List<Criteria> criteria) {
+
+		Assert.notNull(criteria, "Criteria must not be null!");
+
+		return new Criteria(Criteria.this, Combinator.AND, criteria);
+	}
+
 	/**
 	 * Create a new {@link Criteria} and combine it with {@code OR} using the provided {@code column} name.
 	 *
@@ -95,14 +199,43 @@ public CriteriaStep or(String column) {
 
 		Assert.hasText(column, "Column name must not be null or empty!");
 
-		return new DefaultCriteriaStep(SqlIdentifier.unquoted(column)) {
+		SqlIdentifier identifier = SqlIdentifier.unquoted(column);
+		return new DefaultCriteriaStep(identifier) {
 			@Override
 			protected Criteria createCriteria(Comparator comparator, Object value) {
-				return new Criteria(Criteria.this, Combinator.OR, SqlIdentifier.unquoted(column), comparator, value);
+				return new Criteria(Criteria.this, Combinator.OR, Collections.emptyList(), identifier, comparator, value);
 			}
 		};
 	}
 
+	/**
+	 * Create a new {@link Criteria} and combine it as group with {@code OR} using the provided {@link Criteria} group.
+	 *
+	 * @param criteria criteria object.
+	 * @return a new {@link Criteria} object.
+	 * @since 1.1
+	 */
+	public Criteria or(Criteria criteria) {
+
+		Assert.notNull(criteria, "Criteria must not be null!");
+
+		return or(Collections.singletonList(criteria));
+	}
+
+	/**
+	 * Create a new {@link Criteria} and combine it as group with {@code OR} using the provided {@link Criteria} group.
+	 *
+	 * @param criteria criteria object.
+	 * @return a new {@link Criteria} object.
+	 * @since 1.1
+	 */
+	public Criteria or(List<Criteria> criteria) {
+
+		Assert.notNull(criteria, "Criteria must not be null!");
+
+		return new Criteria(Criteria.this, Combinator.OR, criteria);
+	}
+
 	/**
 	 * @return the previous {@link Criteria} object. Can be {@literal null} if there is no previous {@link Criteria}.
 	 * @see #hasPrevious()
@@ -119,6 +252,56 @@ boolean hasPrevious() {
 		return previous != null;
 	}
 
+	/**
+	 * @return {@literal true} if this {@link Criteria} is empty.
+	 * @since 1.1
+	 */
+	public boolean isEmpty() {
+
+		if (!doIsEmpty()) {
+			return false;
+		}
+
+		Criteria parent = this.previous;
+
+		while (parent != null) {
+
+			if (!parent.doIsEmpty()) {
+				return false;
+			}
+
+			parent = parent.previous;
+		}
+
+		return true;
+	}
+
+	private boolean doIsEmpty() {
+
+		if (this.comparator == Comparator.INITIAL) {
+			return true;
+		}
+
+		if (this.column != null) {
+			return false;
+		}
+
+		for (Criteria criteria : group) {
+			if (!criteria.isEmpty()) {
+				return false;
+			}
+		}
+
+		return true;
+	}
+
+	/**
+	 * @return {@literal true} if this {@link Criteria} is empty.
+	 */
+	boolean isGroup() {
+		return !this.group.isEmpty();
+	}
+
 	/**
 	 * @return {@link Combinator} to combine this criteria with a previous one.
 	 */
@@ -126,9 +309,14 @@ Combinator getCombinator() {
 		return combinator;
 	}
 
+	List<Criteria> getGroup() {
+		return group;
+	}
+
 	/**
 	 * @return the column/property name.
 	 */
+	@Nullable
 	SqlIdentifier getColumn() {
 		return column;
 	}
@@ -136,6 +324,7 @@ SqlIdentifier getColumn() {
 	/**
 	 * @return {@link Comparator}.
 	 */
+	@Nullable
 	Comparator getComparator() {
 		return comparator;
 	}
@@ -149,7 +338,7 @@ Object getValue() {
 	}
 
 	enum Comparator {
-		EQ, NEQ, LT, LTE, GT, GTE, IS_NULL, IS_NOT_NULL, LIKE, NOT_IN, IN,
+		INITIAL, EQ, NEQ, LT, LTE, GT, GTE, IS_NULL, IS_NOT_NULL, LIKE, NOT_IN, IN,
 	}
 
 	enum Combinator {
@@ -240,13 +429,11 @@ public interface CriteriaStep {
 
 		/**
 		 * Creates a {@link Criteria} using {@code IS NULL}.
-		 *
 		 */
 		Criteria isNull();
 
 		/**
 		 * Creates a {@link Criteria} using {@code IS NOT NULL}.
-		 *
 		 */
 		Criteria isNotNull();
 	}
diff --git a/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java b/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java
index 8170646f..52547560 100644
--- a/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java
+++ b/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java
@@ -195,9 +195,22 @@ public BoundCondition getMappedObject(BindMarkers markers, Criteria criteria, Ta
 		Assert.notNull(criteria, "Criteria must not be null!");
 		Assert.notNull(table, "Table must not be null!");
 
-		Criteria current = criteria;
 		MutableBindings bindings = new MutableBindings(markers);
 
+		if (criteria.isEmpty()) {
+			throw new IllegalArgumentException("Cannot map empty Criteria");
+		}
+
+		Condition mapped = unroll(criteria, table, entity, bindings);
+
+		return new BoundCondition(bindings, mapped);
+	}
+
+	private Condition unroll(Criteria criteria, Table table, @Nullable RelationalPersistentEntity<?> entity,
+			MutableBindings bindings) {
+
+		Criteria current = criteria;
+
 		// reverse unroll criteria chain
 		Map<Criteria, Criteria> forwardChain = new HashMap<>();
 
@@ -210,25 +223,83 @@ public BoundCondition getMappedObject(BindMarkers markers, Criteria criteria, Ta
 		Condition mapped = getCondition(current, bindings, table, entity);
 		while (forwardChain.containsKey(current)) {
 
-			Criteria nextCriteria = forwardChain.get(current);
+			Criteria criterion = forwardChain.get(current);
+			Condition result = null;
 
-			if (nextCriteria.getCombinator() == Combinator.AND) {
-				mapped = mapped.and(getCondition(nextCriteria, bindings, table, entity));
+			Condition condition = getCondition(criterion, bindings, table, entity);
+			if (condition != null) {
+				result = combine(criterion, mapped, criterion.getCombinator(), condition);
 			}
 
-			if (nextCriteria.getCombinator() == Combinator.OR) {
-				mapped = mapped.or(getCondition(nextCriteria, bindings, table, entity));
+			if (result != null) {
+				mapped = result;
 			}
+			current = criterion;
+		}
 
-			current = nextCriteria;
+		if (mapped == null) {
+			throw new IllegalStateException("Cannot map empty Criteria");
 		}
 
-		return new BoundCondition(bindings, mapped);
+		return mapped;
+	}
+
+	@Nullable
+	private Condition unrollGroup(List<Criteria> criteria, Table table, Combinator combinator,
+			@Nullable RelationalPersistentEntity<?> entity, MutableBindings bindings) {
+
+		Condition mapped = null;
+		for (Criteria criterion : criteria) {
+
+			if (criterion.isEmpty()) {
+				continue;
+			}
+
+			Condition condition = unroll(criterion, table, entity, bindings);
+
+			mapped = combine(criterion, mapped, combinator, condition);
+		}
+
+		return mapped;
 	}
 
+	@Nullable
 	private Condition getCondition(Criteria criteria, MutableBindings bindings, Table table,
 			@Nullable RelationalPersistentEntity<?> entity) {
 
+		if (criteria.isEmpty()) {
+			return null;
+		}
+
+		if (criteria.isGroup()) {
+
+			Condition condition = unrollGroup(criteria.getGroup(), table, criteria.getCombinator(), entity, bindings);
+
+			return condition == null ? null : Conditions.nest(condition);
+		}
+
+		return mapCondition(criteria, bindings, table, entity);
+	}
+
+	private Condition combine(Criteria criteria, @Nullable Condition currentCondition, Combinator combinator,
+			Condition nextCondition) {
+
+		if (currentCondition == null) {
+			currentCondition = nextCondition;
+		} else if (combinator == Combinator.AND) {
+			currentCondition = currentCondition.and(nextCondition);
+		} else if (combinator == Combinator.OR) {
+			currentCondition = currentCondition.or(nextCondition);
+		} else {
+			throw new IllegalStateException("Combinator " + criteria.getCombinator() + " not supported");
+		}
+
+		return currentCondition;
+	}
+
+	private Condition mapCondition(Criteria criteria, MutableBindings bindings, Table table,
+			@Nullable RelationalPersistentEntity<?> entity) {
+
 		Field propertyField = createPropertyField(entity, criteria.getColumn(), this.mappingContext);
 		Column column = table.column(propertyField.getMappedColumnName());
 		TypeInformation<?> actualType = propertyField.getTypeHint().getRequiredActualType();
diff --git a/src/test/java/org/springframework/data/r2dbc/query/CriteriaUnitTests.java b/src/test/java/org/springframework/data/r2dbc/query/CriteriaUnitTests.java
index 887585ba..16b88fb4 100644
--- a/src/test/java/org/springframework/data/r2dbc/query/CriteriaUnitTests.java
+++ b/src/test/java/org/springframework/data/r2dbc/query/CriteriaUnitTests.java
@@ -32,6 +32,27 @@
  */
 public class CriteriaUnitTests {
 
+	@Test // gh-289
+	public void fromCriteria() {
+
+		Criteria nested1 = where("foo").isNotNull();
+		Criteria nested2 = where("foo").isNull();
+		Criteria criteria = Criteria.from(nested1, nested2);
+
+		assertThat(criteria.isGroup()).isTrue();
+		assertThat(criteria.getGroup()).containsExactly(nested1, nested2);
+		assertThat(criteria.getPrevious()).isEqualTo(Criteria.empty());
+	}
+
+	@Test // gh-289
+	public void fromCriteriaOptimized() {
+
+		Criteria nested = where("foo").is("bar").and("baz").isNotNull();
+		Criteria criteria = Criteria.from(nested);
+
+		assertThat(criteria).isSameAs(nested);
+	}
+
 	@Test // gh-64
 	public void andChainedCriteria() {
 
@@ -50,6 +71,23 @@ public void andChainedCriteria() {
 		assertThat(criteria.getValue()).isEqualTo("bar");
 	}
 
+	@Test // gh-289
+	public void andGroupedCriteria() {
+
+		Criteria criteria = where("foo").is("bar").and(where("foo").is("baz"));
+
+		assertThat(criteria.isGroup()).isTrue();
+		assertThat(criteria.getGroup()).hasSize(1);
+		assertThat(criteria.getGroup().get(0).getColumn()).isEqualTo(SqlIdentifier.unquoted("foo"));
+		assertThat(criteria.getCombinator()).isEqualTo(Combinator.AND);
+
+		criteria = criteria.getPrevious();
+
+		assertThat(criteria.getColumn()).isEqualTo(SqlIdentifier.unquoted("foo"));
+		assertThat(criteria.getComparator()).isEqualTo(Comparator.EQ);
+		assertThat(criteria.getValue()).isEqualTo("bar");
+	}
+
 	@Test // gh-64
 	public void orChainedCriteria() {
 
@@ -64,6 +102,23 @@ public void orChainedCriteria() {
 		assertThat(criteria.getValue()).isEqualTo("bar");
 	}
 
+	@Test // gh-289
+	public void orGroupedCriteria() {
+
+		Criteria criteria = where("foo").is("bar").or(where("foo").is("baz"));
+
+		assertThat(criteria.isGroup()).isTrue();
+		assertThat(criteria.getGroup()).hasSize(1);
+		assertThat(criteria.getGroup().get(0).getColumn()).isEqualTo(SqlIdentifier.unquoted("foo"));
+		assertThat(criteria.getCombinator()).isEqualTo(Combinator.OR);
+
+		criteria = criteria.getPrevious();
+
+		assertThat(criteria.getColumn()).isEqualTo(SqlIdentifier.unquoted("foo"));
+		assertThat(criteria.getComparator()).isEqualTo(Comparator.EQ);
+		assertThat(criteria.getValue()).isEqualTo("bar");
+	}
+
 	@Test // gh-64
 	public void shouldBuildEqualsCriteria() {
 
diff --git a/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java b/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java
index d3ea3580..d732a1ee 100644
--- a/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java
+++ b/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java
@@ -19,6 +19,8 @@
 import static org.mockito.Mockito.*;
 import static org.springframework.data.domain.Sort.Order.*;
 
+import java.util.Collections;
+
 import org.junit.Test;
 
 import org.springframework.data.domain.Sort;
@@ -45,6 +47,74 @@ public class QueryMapperUnitTests {
 	QueryMapper mapper = new QueryMapper(PostgresDialect.INSTANCE, converter);
 	BindTarget bindTarget = mock(BindTarget.class);
 
+	@Test // gh-289
+	public void shouldNotMapEmptyCriteria() {
+
+		Criteria criteria = Criteria.empty();
+
+		assertThatIllegalArgumentException().isThrownBy(() -> map(criteria));
+	}
+
+	@Test // gh-289
+	public void shouldNotMapEmptyAndCriteria() {
+
+		Criteria criteria = Criteria.empty().and(Collections.emptyList());
+
+		assertThatIllegalArgumentException().isThrownBy(() -> map(criteria));
+	}
+
+	@Test // gh-289
+	public void shouldNotMapEmptyNestedCriteria() {
+
+		Criteria criteria = Criteria.empty().and(Collections.emptyList()).and(Criteria.empty().and(Criteria.empty()));
+
+		assertThat(criteria.isEmpty()).isTrue();
+		assertThatIllegalArgumentException().isThrownBy(() -> map(criteria));
+	}
+
+	@Test // gh-289
+	public void shouldMapSomeNestedCriteria() {
+
+		Criteria criteria = Criteria.empty().and(Collections.emptyList())
+				.and(Criteria.empty().and(Criteria.where("name").is("Hank")));
+
+		assertThat(criteria.isEmpty()).isFalse();
+
+		BoundCondition bindings = map(criteria);
+
+		assertThat(bindings.getCondition().toString()).isEqualTo("((person.name = ?[$1]))");
+	}
+
+	@Test // gh-289
+	public void shouldMapNestedGroup() {
+
+		Criteria initial = Criteria.empty();
+
+		Criteria criteria = initial.and(Criteria.where("name").is("Foo")).and(Criteria.where("name").is("Bar").or("age")
+				.lessThan(49).or(Criteria.where("name").not("Bar").and("age").greaterThan(49)));
+
+		assertThat(criteria.isEmpty()).isFalse();
+
+		BoundCondition bindings = map(criteria);
+
+		assertThat(bindings.getCondition().toString()).isEqualTo(
+				"(person.name = ?[$1]) AND (person.name = ?[$2] OR person.age < ?[$3] OR (person.name != ?[$4] AND person.age > ?[$5]))");
+	}
+
+	@Test // gh-289
+	public void shouldMapFrom() {
+
+		Criteria criteria = Criteria.from(Criteria.where("name").is("Foo"))
+				.and(Criteria.where("name").is("Bar").or("age").lessThan(49));
+
+		assertThat(criteria.isEmpty()).isFalse();
+
+		BoundCondition bindings = map(criteria);
+
+		assertThat(bindings.getCondition().toString())
+				.isEqualTo("person.name = ?[$1] AND (person.name = ?[$2] OR person.age < ?[$3])");
+	}
+
 	@Test // gh-64
 	public void shouldMapSimpleCriteria() {