lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Robert Muir (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (SOLR-3251) dynamically add fields to schema
Date Wed, 17 Apr 2013 15:51:17 GMT

    [ https://issues.apache.org/jira/browse/SOLR-3251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13634150#comment-13634150
] 

Robert Muir commented on SOLR-3251:
-----------------------------------

+++ solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java	(working copy)
@@ -120,7 +120,6 @@
{code}
 
   private static Logger log = LoggerFactory.getLogger(SolrIndexSearcher.class);
   private final SolrCore core;
-  private final IndexSchema schema;
{code}

Are we sure SolrIndexSearcher cannot just take a snapshot of the schema and just use that?
After all, it represents an immutable point-in-time of the index.
Requests already have indexsearchers assigned to them, so req.getSchema() could just forward
to getSearcher().getSchema(). Then the patch would get a lot
smaller and so much search code would be simpler and probably require no code or API changes
at all to work.

The special cases like real-time get could continue to just call getLatestSchema() from the
core.

This seems to cause a lot of general problems throughout the patch (and require lots of search
code to become more complex, taking additional IndexSchema parameters). After making it halfway
thru the patch, I think doing a small change here might fix 90% of my other comments (but
im including them anyway here, sorry if thats confusing, just want
to reduce the number of iterations hopefully)


solr/core/src/java/org/apache/solr/handler/component/FieldFacetStats.java
{code}
+        facetStats.accumulate(searcher.getCore().getLatestSchema(), value, count);
{code}
Shouldn't this be using the one from the request instead of the moving target?


Index: solr/core/src/java/org/apache/solr/handler/component/QueryElevationComponent.java?
{code}
   @Override
   public void inform(SolrCore core) {
-    String a = initArgs.get(FIELD_TYPE);
-    if (a != null) {
-      FieldType ft = core.getSchema().getFieldTypes().get(a);
+    IndexSchema schema = core.getLatestSchema();
{code}


Index: solr/core/src/java/org/apache/solr/handler/component/SpellCheckComponent.java
{code}
+        IndexSchema schema = core.getLatestSchema();
{code}

These look scary, but seem ok since it only uses the fieldtype (which cannot yet change).

Maybe add a comment just so its clear (especially in case the fieldtype becomes changeable
later)?


+++ solr/core/src/java/org/apache/solr/handler/component/StatsComponent.java	(working copy)
{code}
     boolean isShard = params.getBool(ShardParams.IS_SHARD, false);
     if (null != statsFs) {
+      IndexSchema schema = searcher.getCore().getLatestSchema();

   public NamedList<?> getFieldCacheStats(String fieldName, String[] facet) throws IOException
{
-    final SchemaField sf = searcher.getSchema().getField(fieldName);
+    IndexSchema schema = searcher.getCore().getLatestSchema();
+    final SchemaField sf = schema.getField(fieldName);
{code}

This should be using the one from the request instead of the moving target.


Index: solr/core/src/java/org/apache/solr/handler/loader/CSVLoaderBase.java
{code}
-    schema = req.getSchema();
+    core = req.getCore();
     this.literals = new HashMap<SchemaField, String>();
 
     templateAdd = new AddUpdateCommand(req);
@@ -244,6 +245,7 @@
     CSVLoaderBase.FieldAdder adder = new CSVLoaderBase.FieldAdder();
     CSVLoaderBase.FieldAdder adderKeepEmpty = new CSVLoaderBase.FieldAdderEmpty();
 
+    IndexSchema schema = core.getLatestSchema();
{code}
Is this right? Why not use the one from the request? If there is a reason (which i dont see/understand),
can we add a comment?


Index: solr/core/src/java/org/apache/solr/schema/SchemaField.java
{code}
   /** Declared field property overrides */
-  Map<String,String> args = Collections.emptyMap();
+  Map<String,?> args = Collections.emptyMap();

-  static SchemaField create(String name, FieldType ft, Map<String,String> props) {
+  static SchemaField create(String name, FieldType ft, Map<String,?> props) {
{code}

Why are we losing type safety here? I'm now unclear on what can be in this properties map...


Index: solr/core/src/java/org/apache/solr/search/Grouping.java
{code}
@@ -775,6 +776,7 @@
       // handle case of rows=0
       if (numGroups == 0) return;
 
+      IndexSchema schema = searcher.getCore().getLatestSchema();
{code}
I don't think this should be using a moving target


+++ solr/core/src/java/org/apache/solr/search/JoinQParserPlugin.java	(working copy)
{code}
-      String prefixStr = TrieField.getMainValuePrefix(fromSearcher.getSchema().getFieldType(fromField));
+      String prefixStr = TrieField.getMainValuePrefix(fromSearcher.getCore().getLatestSchema().getFieldType(fromField));
{code}
Nor this

                
> dynamically add fields to schema
> --------------------------------
>
>                 Key: SOLR-3251
>                 URL: https://issues.apache.org/jira/browse/SOLR-3251
>             Project: Solr
>          Issue Type: New Feature
>            Reporter: Yonik Seeley
>            Assignee: Steve Rowe
>             Fix For: 4.3, 5.0
>
>         Attachments: SOLR-3251.patch, SOLR-3251.patch, SOLR-3251.patch, SOLR-3251.patch,
SOLR-3251.patch, SOLR-3251.patch
>
>
> One related piece of functionality needed for SOLR-3250 is the ability to dynamically
add a field to the schema.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message