You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2017/01/08 21:27:38 UTC
[2/2] lucene-solr:jira/solr-5944: SOLR-5944: remove explicit
default=0 from inplace fields in schema,
and update any tests that were implicitly assuming that default would exist
SOLR-5944: remove explicit default=0 from inplace fields in schema, and update any tests that were implicitly assuming that default would exist
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/f8b966cd
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/f8b966cd
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/f8b966cd
Branch: refs/heads/jira/solr-5944
Commit: f8b966cd7e732619fa8b1f7dfc1f0a1891fb3613
Parents: 7a50b74
Author: Chris Hostetter <ho...@apache.org>
Authored: Sun Jan 8 14:27:29 2017 -0700
Committer: Chris Hostetter <ho...@apache.org>
Committed: Sun Jan 8 14:27:29 2017 -0700
----------------------------------------------------------------------
.../collection1/conf/schema-inplace-updates.xml | 13 +++-------
.../solr/update/TestInPlaceUpdatesDistrib.java | 15 ++++++-----
.../update/TestInPlaceUpdatesStandalone.java | 27 +++++++++++++++-----
.../org/apache/solr/update/UpdateLogTest.java | 5 +---
4 files changed, 34 insertions(+), 26 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f8b966cd/solr/core/src/test-files/solr/collection1/conf/schema-inplace-updates.xml
----------------------------------------------------------------------
diff --git a/solr/core/src/test-files/solr/collection1/conf/schema-inplace-updates.xml b/solr/core/src/test-files/solr/collection1/conf/schema-inplace-updates.xml
index b4084ea..b3e1f38 100644
--- a/solr/core/src/test-files/solr/collection1/conf/schema-inplace-updates.xml
+++ b/solr/core/src/test-files/solr/collection1/conf/schema-inplace-updates.xml
@@ -26,13 +26,8 @@
<field name="_version_" type="long" indexed="false" stored="false" docValues="true" />
<!-- specific schema fields for dv in-place updates -->
- <!-- nocommit: tests should be able to pass w/o these fields having default="0" -->
- <!-- nocommit: if any tests that actually depend/use the the default field value,
- nocommit: then they should be changed to use inplace_updatable_float_with_def
- nocommit: (or inplace_updatable_int_with_def) and assume the non-0 default value
- nocommit: so we don't risk confusion with an *implicit* (in code) default of 0 -->
- <field name="inplace_updatable_float" type="float" indexed="false" stored="false" docValues="true" default="0"/>
- <field name="inplace_updatable_int" type="int" indexed="false" stored="false" docValues="true" default="0"/>
+ <field name="inplace_updatable_float" type="float" indexed="false" stored="false" docValues="true" />
+ <field name="inplace_updatable_int" type="int" indexed="false" stored="false" docValues="true" />
<field name="inplace_updatable_float_with_default"
type="float" indexed="false" stored="false" docValues="true" default="42.0"/>
@@ -55,11 +50,11 @@
<copyField source="id" dest="id_field_copy_that_does_not_support_in_place_update_s"/>
<!-- copyfield1: src and dest are both updatable -->
- <field name="copyfield1_src__both_updateable" type="int" indexed="false" stored="false" docValues="true" default="0"/>
+ <field name="copyfield1_src__both_updateable" type="int" indexed="false" stored="false" docValues="true" />
<copyField source="copyfield1_src__both_updateable" dest="copyfield1_dest__both_updatable_i_dvo"/>
<!-- copyfield2: src is updatable but dest is not -->
- <field name="copyfield2_src__only_src_updatable" type="int" indexed="false" stored="false" docValues="true" default="0"/>
+ <field name="copyfield2_src__only_src_updatable" type="int" indexed="false" stored="false" docValues="true" />
<copyField source="copyfield2_src__only_src_updatable" dest="copyfield2_dest__only_src_updatable_i"/>
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f8b966cd/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
index 1ee1cae..c75dd20 100644
--- a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
+++ b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
@@ -108,13 +108,11 @@ public class TestInPlaceUpdatesDistrib extends AbstractFullDistribZkTestBase {
// sanity check no one broke the assumptions we make about our schema
checkExpectedSchemaField(map("name", "inplace_updatable_int",
"type","int",
- "default","0", // nocommit: we have no reason to depend on this
"stored",Boolean.FALSE,
"indexed",Boolean.FALSE,
"docValues",Boolean.TRUE));
checkExpectedSchemaField(map("name", "inplace_updatable_float",
"type","float",
- "default","0", // nocommit: we have no reason to depend on this
"stored",Boolean.FALSE,
"indexed",Boolean.FALSE,
"docValues",Boolean.TRUE));
@@ -246,11 +244,7 @@ public class TestInPlaceUpdatesDistrib extends AbstractFullDistribZkTestBase {
ids.add(id);
}
- // nocommit: is it definitely safe to use "true" here?
- // nocommit: once the default="0" from the schema is removed, then some docs won't have an init value,
- // nocommit: will that mean the docid will change on 1st set? if so: it will break luceneDocids asserts
- // nocommit: we can switch to false to ensure every doc starts with a value if need be
- buildRandomIndex(101.0F, true, ids);
+ buildRandomIndex(101.0F, false, ids);
List<Integer> luceneDocids = new ArrayList<>(numDocs);
List<Float> valuesList = new ArrayList<Float>(numDocs);
@@ -399,6 +393,13 @@ public class TestInPlaceUpdatesDistrib extends AbstractFullDistribZkTestBase {
currentVersion = buildRandomIndex(100).get(0);
assertTrue(currentVersion > version);
+
+ // do an initial (non-inplace) update to ensure both the float & int fields we care about have (any) value
+ // that way all subsequent atomic updates will be inplace
+ currentVersion = addDocAndGetVersion("id", 100,
+ "inplace_updatable_float", map("set", random().nextFloat()),
+ "inplace_updatable_int", map("set", random().nextInt()));
+ LEADER.commit();
// get the internal docids of id=100 document from the three replicas
List<Integer> docids = getInternalDocIds("100");
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f8b966cd/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
index cfadc6f..3f155e1 100644
--- a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
+++ b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesStandalone.java
@@ -113,9 +113,9 @@ public class TestInPlaceUpdatesStandalone extends SolrTestCaseJ4 {
@Test
public void testUpdatingDocValues() throws Exception {
- long version1 = addAndGetVersion(sdoc("id", "1", "title_s", "first"), null);
- long version2 = addAndGetVersion(sdoc("id", "2", "title_s", "second"), null);
- long version3 = addAndGetVersion(sdoc("id", "3", "title_s", "third"), null);
+ long version1 = addAndGetVersion(sdoc("id", "1", "title_s", "first", "inplace_updatable_float", 41), null);
+ long version2 = addAndGetVersion(sdoc("id", "2", "title_s", "second", "inplace_updatable_float", 42), null);
+ long version3 = addAndGetVersion(sdoc("id", "3", "title_s", "third", "inplace_updatable_float", 43), null);
assertU(commit("softCommit", "false"));
assertQ(req("q", "*:*"), "//*[@numFound='3']");
@@ -281,7 +281,7 @@ public class TestInPlaceUpdatesStandalone extends SolrTestCaseJ4 {
@Test
public void testUpdateTwoDifferentFields() throws Exception {
- long version1 = addAndGetVersion(sdoc("id", "1", "title_s", "first"), null);
+ long version1 = addAndGetVersion(sdoc("id", "1", "title_s", "first", "inplace_updatable_float", 42), null);
assertU(commit("softCommit", "false"));
assertQ(req("q", "*:*"), "//*[@numFound='1']");
@@ -1057,13 +1057,28 @@ public class TestInPlaceUpdatesStandalone extends SolrTestCaseJ4 {
"new_updateable_int_i_dvo", map("set", 10)));
assertTrue(inPlaceUpdatedFields.contains("new_updateable_int_i_dvo"));
+ // for copy fields, regardless of wether the source & target support inplace updates,
+ // it won't be inplace if the DVs don't exist yet...
+ assertTrue("inplace fields should be empty when doc has no copyfield src values yet",
+ callComputeInPlaceUpdateableFields(sdoc("id", "1", "_version_", 42L,
+ "copyfield1_src__both_updateable", map("set", 1),
+ "copyfield2_src__only_src_updatable", map("set", 2))).isEmpty());
+
+ // now add a doc that *does* have the src field for each copyfield...
+ addAndGetVersion(sdoc("id", "3",
+ "copyfield1_src__both_updateable", -13,
+ "copyfield2_src__only_src_updatable", -15), params());
+ if (random().nextBoolean()) {
+ assertU(commit("softCommit", "false"));
+ }
+
// If a supported dv field has a copyField target which is supported, it should be an in-place update
- inPlaceUpdatedFields = callComputeInPlaceUpdateableFields(sdoc("id", "1", "_version_", 42L,
+ inPlaceUpdatedFields = callComputeInPlaceUpdateableFields(sdoc("id", "3", "_version_", 42L,
"copyfield1_src__both_updateable", map("set", 10)));
assertTrue(inPlaceUpdatedFields.contains("copyfield1_src__both_updateable"));
// If a supported dv field has a copyField target which is not supported, it should not be an in-place update
- inPlaceUpdatedFields = callComputeInPlaceUpdateableFields(sdoc("id", "1", "_version_", 42L,
+ inPlaceUpdatedFields = callComputeInPlaceUpdateableFields(sdoc("id", "3", "_version_", 42L,
"copyfield2_src__only_src_updatable", map("set", 10)));
assertTrue(inPlaceUpdatedFields.isEmpty());
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f8b966cd/solr/core/src/test/org/apache/solr/update/UpdateLogTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/update/UpdateLogTest.java b/solr/core/src/test/org/apache/solr/update/UpdateLogTest.java
index 7d08729..458097e 100644
--- a/solr/core/src/test/org/apache/solr/update/UpdateLogTest.java
+++ b/solr/core/src/test/org/apache/solr/update/UpdateLogTest.java
@@ -166,11 +166,8 @@ public class UpdateLogTest extends SolrTestCaseJ4 {
// sanity check that the update log has one document, and RTG returns the document
assertEquals(1, ulog.map.size());
assertJQ(req("qt","/get", "id","1")
- , "=={'doc':{ 'id':'1', 'val1_i_dvo':3, "
+ , "=={'doc':{ 'id':'1', 'val1_i_dvo':3, '_version_':102, 'title_s':'title1', "
// fields with default values
- + "'_version_':102, 'title_s':'title1', "
- + "'copyfield1_src__both_updateable':0, 'inplace_updatable_float':0.0,"
- + "'copyfield2_src__only_src_updatable':0, 'inplace_updatable_int':0, "
+ "'inplace_updatable_int_with_default':666, 'inplace_updatable_float_with_default':42.0}}");
boolean dbq = random().nextBoolean();