From 4085e20e249d3fa29a996ed5b85ccda1dc7cdc8a Mon Sep 17 00:00:00 2001 From: Igor Kim Date: Thu, 29 Aug 2019 03:14:36 +0500 Subject: [PATCH] Fix Concurrent modification on non-grouping query with aggregates Bug: T172113 Change-Id: I0605e725c45f35f5d2e24f4fcc34560e844d4edf --- .../rdf/sparql/ast/StaticAnalysis.java | 45 ++++++----- .../rdf/sail/webapp/Test_Ticket_T172113.java | 74 +++++++++++++++++++ 2 files changed, 102 insertions(+), 17 deletions(-) create mode 100644 bigdata-sails-test/src/test/java/com/bigdata/rdf/sail/webapp/Test_Ticket_T172113.java diff --git a/bigdata-core/bigdata-rdf/src/java/com/bigdata/rdf/sparql/ast/StaticAnalysis.java b/bigdata-core/bigdata-rdf/src/java/com/bigdata/rdf/sparql/ast/StaticAnalysis.java index 1f139fd5ae..ab6101bd88 100644 --- a/bigdata-core/bigdata-rdf/src/java/com/bigdata/rdf/sparql/ast/StaticAnalysis.java +++ b/bigdata-core/bigdata-rdf/src/java/com/bigdata/rdf/sparql/ast/StaticAnalysis.java @@ -2047,29 +2047,20 @@ public static INeedsMaterialization.Requirement gatherVarsToMaterialize( final INeedsMaterialization bop2 = (INeedsMaterialization) bop; - final Set> t = getVarsFromArguments(bop); + final Collection> varsFromArguments = getVarsFromArguments(bop); + // Need to collect vars into a new set, as adding referenced vars into the original one (varsFromArguments) + // results in ConcurrentModificationException. The use of recursive method collectVarsFromExpressions also + // fixes references of derived vars, for example ?a+?b AS ?c, 1+?c AS ?d + // Ref: Test_Ticket_T172113 + final Collection> t = new LinkedHashSet<>(varsFromArguments); // https://jira.blazegraph.com/browse/BLZG-2083 (str() produces NotMaterializedException when using group by/sample) // NotMaterializedException can be thrown by a function referenced in group by, to avoid that all variables referenced // in group by expressions should be materialized, as MemoryGroupByOp needs real IVs. To avoid excessive adding of // unneeded variables, varMap is provided as a reference of variables which might need materialization. if (varMap != null) { - - for (IVariable key : t) { - - IValueExpression expr = varMap.get(key); - - if (expr != null) { - - Set> vars = getVarsFromArguments(expr); - - t.addAll(vars); - - } - - } - - } + collectVarsFromExpressions(varMap, varsFromArguments, t); + } if (t.size() > 0) { @@ -2096,6 +2087,26 @@ public static INeedsMaterialization.Requirement gatherVarsToMaterialize( } + @SuppressWarnings({ "rawtypes" }) + private static void collectVarsFromExpressions(Map, IValueExpression> varMap, + Collection> varsFromArguments, Collection> result) { + for (IVariable key : varsFromArguments) { + + IValueExpression expr = varMap.get(key); + + if (expr != null) { + + Set> vars = getVarsFromArguments(expr); + + collectVarsFromExpressions(varMap, vars, result); + + result.addAll(vars); + + } + + } + } + @SuppressWarnings({ "rawtypes", "unchecked" }) private static Set> getVarsFromArguments(final BOp c) { diff --git a/bigdata-sails-test/src/test/java/com/bigdata/rdf/sail/webapp/Test_Ticket_T172113.java b/bigdata-sails-test/src/test/java/com/bigdata/rdf/sail/webapp/Test_Ticket_T172113.java new file mode 100644 index 0000000000..e54c136485 --- /dev/null +++ b/bigdata-sails-test/src/test/java/com/bigdata/rdf/sail/webapp/Test_Ticket_T172113.java @@ -0,0 +1,74 @@ +/* +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; version 2 of the License. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program; if not, write to the Free Software +Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +*/ + + +package com.bigdata.rdf.sail.webapp; + +import junit.framework.Test; + +import org.openrdf.query.QueryLanguage; +import org.openrdf.query.TupleQuery; +import org.openrdf.query.TupleQueryResult; + +import com.bigdata.journal.IIndexManager; +import com.bigdata.rdf.sail.webapp.client.RemoteRepository.RemoveOp; + +/** + * ConcurrentModificationException on non-grouping query with aggregates in SELECT. + * @See https://phabricator.wikimedia.org/T172113 + */ +public class Test_Ticket_T172113 extends + AbstractTestNanoSparqlClient { + + public Test_Ticket_T172113() { + + } + + public Test_Ticket_T172113(final String name) { + + super(name); + + } + + public static Test suite() { + + return ProxySuiteHelper.suiteWhenStandalone(Test_Ticket_T172113.class, + "test.*", TestMode.triples + ); + + } + + /** + * Test supposed to check if constants .. will be resolved + */ + public void test_aggregatesWithoutGroupBy_T172113() throws Exception { + // Clear DB per task description (Executing the query over the empty database) + m_repo.remove(new RemoveOp(null, null, null)); + String query = "SELECT (COUNT(*) AS ?a) (COUNT(?x) AS ?b) (?b/?a AS ?r) { BIND(1 AS ?x)}"; + final TupleQuery tq = m_repo.getBigdataSailRemoteRepository().getConnection().prepareTupleQuery(QueryLanguage.SPARQL, query, null); + final TupleQueryResult tqr = tq.evaluate(); + try { + int count = 0; + while (tqr.hasNext()) { + System.out.println(tqr.next()); + count++; + } + assertEquals(1,count); // asserting successful execution of the query, as it was failing while parsing + } finally { + tqr.close(); + } + } + +}