You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ge...@apache.org on 2020/09/22 14:29:17 UTC
[lucene-solr] branch branch_8x updated: SOLR-14859: Set fieldType
defaults for DateRangeField
This is an automated email from the ASF dual-hosted git repository.
gerlowskija pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new 52156bb SOLR-14859: Set fieldType defaults for DateRangeField
52156bb is described below
commit 52156bb99faed2a8e3ee8c379c6d8df2c81cf5c5
Author: Jason Gerlowski <ja...@lucidworks.com>
AuthorDate: Thu Sep 17 11:42:02 2020 -0400
SOLR-14859: Set fieldType defaults for DateRangeField
AbstractSpatialPrefixTreeFieldType (and its children) create index
fields based on a prototype with options frozen in PrefixTreeStrategy,
regardless of options specified in the schema. This works fine most of
the time, but causes problems when QParsers or other query optimization
logic makes decisions based on these options (which are potentially out
of sync with the underlying index data). Most commonly this causes
issues with "exists" (e.g. [* TO *]) queries.
This commit enforces fieldType defaults that line up with the 'hardcoded'
FieldType used by PrefixTreeStrategy. Options on either the fieldType
or the field itself which contradict these defaults will result in
exceptions at schema load/modification time.
---
solr/CHANGES.txt | 3 +
.../schema/AbstractSpatialPrefixTreeFieldType.java | 67 ++++++++++++++++++++++
.../bad-schema-daterangefield-instance-options.xml | 35 +++++++++++
.../bad-schema-daterangefield-type-options.xml | 35 +++++++++++
.../org/apache/solr/schema/BadIndexSchemaTest.java | 5 ++
.../org/apache/solr/schema/DateRangeFieldTest.java | 4 ++
.../solr/schema/SpatialRPTFieldTypeTest.java | 3 +-
7 files changed, 151 insertions(+), 1 deletion(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 3f249bc..e988c47 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -53,6 +53,9 @@ Improvements
* SOLR-14802: geodist: Support most (all?) spatial field types as an argument like LLPSF, SRPTFT, and others.
(Tom Edge, Craig Wrigglesworth)
+* SOLR-14859: DateRangeField now throws errors when invalid field/fieldType options specified; no longer silently accepts incompatible option values
+ (Jason Gerlowski, Chris Hostetter, Munendra S N)
+
Optimizations
---------------------
diff --git a/solr/core/src/java/org/apache/solr/schema/AbstractSpatialPrefixTreeFieldType.java b/solr/core/src/java/org/apache/solr/schema/AbstractSpatialPrefixTreeFieldType.java
index eb7f191..f7b9912 100644
--- a/solr/core/src/java/org/apache/solr/schema/AbstractSpatialPrefixTreeFieldType.java
+++ b/solr/core/src/java/org/apache/solr/schema/AbstractSpatialPrefixTreeFieldType.java
@@ -18,14 +18,18 @@ package org.apache.solr.schema;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
+import java.util.HashMap;
+import java.util.Locale;
import java.util.Map;
import org.apache.commons.io.IOUtils;
import org.apache.lucene.analysis.Analyzer;
+import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.spatial.prefix.PrefixTreeStrategy;
import org.apache.lucene.spatial.prefix.tree.SpatialPrefixTree;
import org.apache.lucene.spatial.prefix.tree.SpatialPrefixTreeFactory;
import org.apache.lucene.spatial.query.SpatialArgsParser;
+import org.apache.solr.common.SolrException;
import org.apache.solr.util.MapListener;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -39,6 +43,24 @@ public abstract class AbstractSpatialPrefixTreeFieldType<T extends PrefixTreeStr
/** @see org.apache.lucene.spatial.prefix.RecursivePrefixTreeStrategy#setDefaultFieldValuesArrayLen(int) */
public static final String DEFAULT_FIELD_VALUES_ARRAY_LEN = "defaultFieldValuesArrayLen";
+ /*
+ * A list of the properties hardcoded within PrefixTreeStrategy.FIELD_TYPE.
+ *
+ * Used primarily for validating that user-provided configurations don't disagree with these invariants. Intentionally
+ * left out of this map is 'tokenized' which is hardcoded to 'true' in PrefixTreeStrategy.FIELD_TYPE, but triggers
+ * unwanted tokenization logic in Solr query parsing.
+ *
+ * @see PrefixTreeStrategy#FIELD_TYPE
+ */
+ public static final Map<String, String> FIELD_TYPE_INVARIANTS = new HashMap<>();
+ static {
+ FIELD_TYPE_INVARIANTS.put("omitNorms", "true");
+ FIELD_TYPE_INVARIANTS.put("termPositions", "false");
+ FIELD_TYPE_INVARIANTS.put("termOffsets", "false");
+ FIELD_TYPE_INVARIANTS.put("omitTermFreqAndPositions", "true");
+ FIELD_TYPE_INVARIANTS.put("omitPositions", "true");
+ }
+
protected SpatialPrefixTree grid;
private Double distErrPct;
private Integer defaultFieldValuesArrayLen;
@@ -46,6 +68,30 @@ public abstract class AbstractSpatialPrefixTreeFieldType<T extends PrefixTreeStr
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
@Override
+ public void setArgs(IndexSchema schema, Map<String, String> args) {
+ for (Map.Entry<String, String> entry : FIELD_TYPE_INVARIANTS.entrySet()) {
+ final String key = entry.getKey();
+ final String hardcodedValue = entry.getValue();
+ final String userConfiguredValue = args.get(entry.getKey());
+
+ if (args.containsKey(key)) {
+ if (userConfiguredValue.equals(hardcodedValue)) {
+ log.warn("FieldType {} does not allow {} to be specified in schema, hardcoded behavior is {}={}",
+ getClass().getSimpleName(), key, key, hardcodedValue);
+ } else {
+ final String message = String.format(Locale.ROOT, "FieldType %s is incompatible with %s=%s; hardcoded " +
+ "behavior is %s=%s. Remove specification in schema",
+ getClass().getSimpleName(), key, userConfiguredValue, key, hardcodedValue);
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, message);
+ }
+ }
+ }
+ args.putAll(FIELD_TYPE_INVARIANTS);
+
+ super.setArgs(schema, args);
+ }
+
+ @Override
protected void init(IndexSchema schema, Map<String, String> args) {
super.init(schema, args);
@@ -71,6 +117,27 @@ public abstract class AbstractSpatialPrefixTreeFieldType<T extends PrefixTreeStr
if (v != null)
defaultFieldValuesArrayLen = Integer.valueOf(v);
}
+
+ /**
+ *
+ * @see #FIELD_TYPE_INVARIANTS
+ */
+ @Override
+ public void checkSchemaField(final SchemaField field) {
+ super.checkSchemaField(field);
+
+ if (! field.omitNorms()) {
+ final String message = String.format(Locale.ROOT, "%s of type %s is incompatible with omitNorms=false; hardcoded " +
+ "behavior is omitNorms=true. Remove specification in schema", field.getName(), getClass().getSimpleName());
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, message);
+ }
+ if (field.indexOptions() != IndexOptions.DOCS) {
+ final String message = String.format(Locale.ROOT,
+ "%s of type %s is incompatible with termFreq or position storage. Remove specification in schema.",
+ field.getName(), getClass().getSimpleName());
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, message);
+ }
+ }
/**
* This analyzer is not actually used for indexing. It is implemented here
diff --git a/solr/core/src/test-files/solr/collection1/conf/bad-schema-daterangefield-instance-options.xml b/solr/core/src/test-files/solr/collection1/conf/bad-schema-daterangefield-instance-options.xml
new file mode 100644
index 0000000..4ee6e39
--- /dev/null
+++ b/solr/core/src/test-files/solr/collection1/conf/bad-schema-daterangefield-instance-options.xml
@@ -0,0 +1,35 @@
+<?xml version="1.0" ?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<schema name="bad-schema-currencyfieldtype-bogus-amount-suffix" version="1.6">
+ <fieldType name="string" class="solr.StrField" multiValued="true"/>
+ <fieldType name="plong" class="solr.LongPointField" docValues="true"/>
+
+ <!-- BEGIN BAD STUFF: bogus amount field suffix -->
+ <fieldType name="daterange" class="solr.DateRangeField" />
+ <!-- END BAD STUFF -->
+
+ <field name="id" type="string" indexed="true" stored="true" multiValued="false"/>
+
+ <field name="daterange_field" type="daterange" omitNorms="false"/>
+
+ <dynamicField name="*_s" type="string" multiValued="false"/>
+ <dynamicField name="*_l" type="plong" multiValued="false"/>
+ <dynamicField name="*_c" type="currency" indexed="true" stored="true"/>
+
+</schema>
diff --git a/solr/core/src/test-files/solr/collection1/conf/bad-schema-daterangefield-type-options.xml b/solr/core/src/test-files/solr/collection1/conf/bad-schema-daterangefield-type-options.xml
new file mode 100644
index 0000000..9489cf1
--- /dev/null
+++ b/solr/core/src/test-files/solr/collection1/conf/bad-schema-daterangefield-type-options.xml
@@ -0,0 +1,35 @@
+<?xml version="1.0" ?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<schema name="bad-schema-currencyfieldtype-bogus-amount-suffix" version="1.6">
+ <fieldType name="string" class="solr.StrField" multiValued="true"/>
+ <fieldType name="plong" class="solr.LongPointField" docValues="true"/>
+
+ <!-- BEGIN BAD STUFF: bogus amount field suffix -->
+ <fieldType name="daterange" class="solr.DateRangeField" omitNorms="false" />
+ <!-- END BAD STUFF -->
+
+ <field name="id" type="string" indexed="true" stored="true" multiValued="false"/>
+
+ <field name="daterange_field" type="daterange"/>
+
+ <dynamicField name="*_s" type="string" multiValued="false"/>
+ <dynamicField name="*_l" type="plong" multiValued="false"/>
+ <dynamicField name="*_c" type="currency" indexed="true" stored="true"/>
+
+</schema>
diff --git a/solr/core/src/test/org/apache/solr/schema/BadIndexSchemaTest.java b/solr/core/src/test/org/apache/solr/schema/BadIndexSchemaTest.java
index c829c17..905868a 100644
--- a/solr/core/src/test/org/apache/solr/schema/BadIndexSchemaTest.java
+++ b/solr/core/src/test/org/apache/solr/schema/BadIndexSchemaTest.java
@@ -186,4 +186,9 @@ public class BadIndexSchemaTest extends AbstractBadConfigTestBase {
public void testSchemaWithDefaultSearchField() throws Exception {
doTest("bad-schema-defaultsearchfield.xml", "Setting defaultSearchField in schema not supported since Solr 7");
}
+
+ public void testDateRangeFieldWithInvalidOptions() throws Exception {
+ doTest("bad-schema-daterangefield-type-options.xml", "FieldType DateRangeField is incompatible with omitNorms=false");
+ doTest("bad-schema-daterangefield-instance-options.xml", "daterange_field of type DateRangeField is incompatible with omitNorms=false");
+ }
}
diff --git a/solr/core/src/test/org/apache/solr/schema/DateRangeFieldTest.java b/solr/core/src/test/org/apache/solr/schema/DateRangeFieldTest.java
index edd25b0..c7b9351 100644
--- a/solr/core/src/test/org/apache/solr/schema/DateRangeFieldTest.java
+++ b/solr/core/src/test/org/apache/solr/schema/DateRangeFieldTest.java
@@ -35,6 +35,7 @@ public class DateRangeFieldTest extends SolrTestCaseJ4 {
assertU(adoc("id", "3", "dateRange", "2020-05-21T12:00:00.000Z/DAY"));//DateMath syntax
assertU(commit());
+
//ensure stored value resolves datemath
assertQ(req("q", "id:1", "fl", "dateRange"), "//result/doc/arr[@name='dateRange']/str[.='2014-05-21T12:00:00Z']");//no 000 ms
assertQ(req("q", "id:2", "fl", "dateRange"), "//result/doc/arr[@name='dateRange']/str[.='[2000 TO 2014-05-21]']");//a range; same
@@ -50,7 +51,10 @@ public class DateRangeFieldTest extends SolrTestCaseJ4 {
assertQ(req("q", "dateRange:[1998 TO 2000}"), xpathMatches(0));//exclusive end, so we barely miss one doc
+
//show without local-params
+ assertQ(req("q", "dateRange:[* TO *]"), xpathMatches(0, 1, 2, 3));
+ assertQ(req("q", "dateRange:*"), xpathMatches(0, 1, 2, 3));
assertQ(req("q", "dateRange:\"2014-05-21T12:00:00.000Z\""), xpathMatches(0, 1, 2));
assertQ(req("q", "dateRange:[1999 TO 2001]"), xpathMatches(0, 2));
}
diff --git a/solr/core/src/test/org/apache/solr/schema/SpatialRPTFieldTypeTest.java b/solr/core/src/test/org/apache/solr/schema/SpatialRPTFieldTypeTest.java
index c780d73..05166ba 100644
--- a/solr/core/src/test/org/apache/solr/schema/SpatialRPTFieldTypeTest.java
+++ b/solr/core/src/test/org/apache/solr/schema/SpatialRPTFieldTypeTest.java
@@ -262,7 +262,8 @@ public class SpatialRPTFieldTypeTest extends AbstractBadConfigTestBase {
}
fieldType.init(oldSchema, rptMap);
fieldType.setTypeName("location_rpt");
- SchemaField newField = new SchemaField("geo", fieldType, SchemaField.STORED | SchemaField.INDEXED, null);
+ SchemaField newField = new SchemaField("geo", fieldType, SchemaField.STORED | SchemaField.INDEXED | SchemaField.OMIT_NORMS | SchemaField.OMIT_TF_POSITIONS,
+ null);
IndexSchema newSchema = oldSchema.addField(newField);
h.getCore().setLatestSchema(newSchema);