Skip to content

Commit

Permalink
Fix bug in index suffix and contains queries with in-tx modified nodes
Browse files Browse the repository at this point in the history
  • Loading branch information
fickludd committed Mar 13, 2018
1 parent 422e2aa commit 67e8219
Show file tree
Hide file tree
Showing 6 changed files with 217 additions and 5 deletions.
@@ -0,0 +1,134 @@
/*
* Copyright (c) 2002-2018 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j 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, either version 3 of the License, or
* (at your option) any later version.
*
* 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, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.internal.kernel.api;

import org.junit.Test;

import java.util.concurrent.TimeUnit;

import org.neo4j.collection.primitive.Primitive;
import org.neo4j.collection.primitive.PrimitiveLongSet;
import org.neo4j.graphdb.GraphDatabaseService;
import org.neo4j.graphdb.Label;
import org.neo4j.graphdb.Node;
import org.neo4j.values.storable.Values;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.junit.Assert.assertThat;
import static org.neo4j.graphdb.Label.label;

public abstract class NodeIndexTransactionStateTestBase<G extends KernelAPIWriteTestSupport>
extends KernelAPIWriteTestBase<G>
{
@Test
public void shouldPerformStringSuffixSearch() throws Exception
{
// given
PrimitiveLongSet expected = Primitive.longSet();
try ( Transaction tx = session.beginTransaction() )
{
expected.add( nodeWithProp( tx, "1suff" ) );
nodeWithProp( tx, "pluff" );
tx.success();
}

createIndex();

// when
try ( Transaction tx = session.beginTransaction() )
{
int label = tx.tokenRead().nodeLabel( "Node" );
int prop = tx.tokenRead().propertyKey( "prop" );
expected.add( nodeWithProp( tx, "2suff" ) );
nodeWithProp( tx, "skruff" );
CapableIndexReference index = tx.schemaRead().index( label, prop );
try ( NodeValueIndexCursor nodes = cursors.allocateNodeValueIndexCursor() )
{
tx.dataRead().nodeIndexSeek( index, nodes, IndexOrder.NONE, IndexQuery.stringSuffix( prop, "suff" ) );
PrimitiveLongSet found = Primitive.longSet();
while ( nodes.next() )
{
found.add( nodes.nodeReference() );
}

assertThat( found, equalTo( expected ) );
}
}
}

@Test
public void shouldPerformStringContainsSearch() throws Exception
{
// given
PrimitiveLongSet expected = Primitive.longSet();
try ( Transaction tx = session.beginTransaction() )
{
expected.add( nodeWithProp( tx, "gnomebat" ) );
nodeWithProp( tx, "fishwombat" );
tx.success();
}

createIndex();

// when
try ( Transaction tx = session.beginTransaction() )
{
int label = tx.tokenRead().nodeLabel( "Node" );
int prop = tx.tokenRead().propertyKey( "prop" );
expected.add( nodeWithProp( tx, "homeopatic" ) );
nodeWithProp( tx, "telephonecompany" );
CapableIndexReference index = tx.schemaRead().index( label, prop );
try ( NodeValueIndexCursor nodes = cursors.allocateNodeValueIndexCursor() )
{
tx.dataRead().nodeIndexSeek( index, nodes, IndexOrder.NONE, IndexQuery.stringContains( prop, "me" ) );
PrimitiveLongSet found = Primitive.longSet();
while ( nodes.next() )
{
found.add( nodes.nodeReference() );
}

assertThat( found, equalTo( expected ) );
}
}
}

private long nodeWithProp( Transaction tx, Object value ) throws Exception
{
Write write = tx.dataWrite();
long node = write.nodeCreate();
write.nodeAddLabel( node, tx.tokenWrite().labelGetOrCreateForName( "Node" ) );
write.nodeSetProperty( node, tx.tokenWrite().propertyKeyGetOrCreateForName( "prop" ), Values.of( value ) );
return node;
}

private void createIndex()
{
try ( org.neo4j.graphdb.Transaction tx = graphDb.beginTx() )
{
graphDb.schema().indexFor( Label.label( "Node" ) ).on( "prop" ).create();
tx.success();
}

try ( org.neo4j.graphdb.Transaction tx = graphDb.beginTx() )
{
graphDb.schema().awaitIndexesOnline( 1, TimeUnit.MINUTES );
}
}
}
Expand Up @@ -88,7 +88,7 @@ public int getPropertyId()
if ( propertyIds.length != 1 )
{
throw new IllegalStateException(
"Single property schema requires one property but had " + propertyIds.length );
"Single property schema required one property but had " + propertyIds.length );
}
return propertyIds[0];
}
Expand Down
Expand Up @@ -37,6 +37,7 @@
import org.neo4j.collection.primitive.PrimitiveLongSet;
import org.neo4j.cursor.Cursor;
import org.neo4j.helpers.collection.Iterables;
import org.neo4j.internal.kernel.api.IndexQuery;
import org.neo4j.internal.kernel.api.exceptions.schema.ConstraintValidationException;
import org.neo4j.internal.kernel.api.schema.LabelSchemaDescriptor;
import org.neo4j.internal.kernel.api.schema.SchemaDescriptor;
Expand Down Expand Up @@ -1014,6 +1015,33 @@ public PrimitiveLongReadableDiffSets indexUpdatesForScan( SchemaIndexDescriptor
return diffs;
}

@Override
public PrimitiveLongReadableDiffSets indexUpdatesForSuffixOrContains( SchemaIndexDescriptor descriptor, IndexQuery query )
{
descriptor.schema().getPropertyId(); // assert single property index

if ( indexUpdates == null )
{
return PrimitiveLongReadableDiffSets.EMPTY;
}
Map<ValueTuple, PrimitiveLongDiffSets> updates = indexUpdates.get( descriptor.schema() );
if ( updates == null )
{
return PrimitiveLongReadableDiffSets.EMPTY;
}
PrimitiveLongDiffSets diffs = new PrimitiveLongDiffSets();
for ( Map.Entry<ValueTuple,PrimitiveLongDiffSets> entry : updates.entrySet() )
{
if ( query.acceptsValue( entry.getKey().getOnlyValue() ) )
{
PrimitiveLongDiffSets diffsets = entry.getValue();
diffs.addAll( diffsets.getAdded().iterator() );
diffs.removeAll( diffsets.getRemoved().iterator() );
}
}
return diffs;
}

@Override
public PrimitiveLongReadableDiffSets indexUpdatesForSeek( SchemaIndexDescriptor descriptor, ValueTuple values )
{
Expand Down
Expand Up @@ -75,19 +75,23 @@ public void initialize( SchemaIndexDescriptor descriptor, IndexProgressor progre
seekQuery( descriptor, query );
break;

case stringSuffix:
case stringContains:
case exists:
scanQuery( descriptor );
break;

case range:
assert query.length == 1;
rangeQuery( descriptor, (IndexQuery.RangePredicate) query[0] );
rangeQuery( descriptor, (IndexQuery.RangePredicate) firstPredicate );
break;

case stringPrefix:
prefixQuery( descriptor, (IndexQuery.StringPrefixPredicate) query[0] );
prefixQuery( descriptor, (IndexQuery.StringPrefixPredicate) firstPredicate );
break;

case stringSuffix:
case stringContains:
assert query.length == 1;
suffixOrContainsQuery( descriptor, firstPredicate );
break;

default:
Expand Down Expand Up @@ -257,6 +261,18 @@ private void scanQuery( SchemaIndexDescriptor descriptor )
}
}

private void suffixOrContainsQuery( SchemaIndexDescriptor descriptor, IndexQuery query )
{
needsValues = true;
if ( read.hasTxStateWithChanges() )
{
TransactionState txState = read.txState();
PrimitiveLongReadableDiffSets changes = txState.indexUpdatesForSuffixOrContains( descriptor, query );
added = changes.augment( emptyIterator() );
removed = removed( txState, changes );
}
}

private void seekQuery( SchemaIndexDescriptor descriptor, IndexQuery[] query )
{
needsValues = false;
Expand Down
Expand Up @@ -25,6 +25,7 @@
import org.neo4j.collection.primitive.PrimitiveLongIterator;
import org.neo4j.collection.primitive.PrimitiveLongResourceIterator;
import org.neo4j.cursor.Cursor;
import org.neo4j.internal.kernel.api.IndexQuery;
import org.neo4j.internal.kernel.api.exceptions.schema.ConstraintValidationException;
import org.neo4j.internal.kernel.api.schema.SchemaDescriptor;
import org.neo4j.internal.kernel.api.schema.constraints.ConstraintDescriptor;
Expand Down Expand Up @@ -140,6 +141,8 @@ public interface ReadableTransactionState

PrimitiveLongReadableDiffSets indexUpdatesForScan( SchemaIndexDescriptor index );

PrimitiveLongReadableDiffSets indexUpdatesForSuffixOrContains( SchemaIndexDescriptor index, IndexQuery query );

PrimitiveLongReadableDiffSets indexUpdatesForSeek( SchemaIndexDescriptor index, ValueTuple values );

PrimitiveLongReadableDiffSets indexUpdatesForRangeSeek( SchemaIndexDescriptor index, ValueGroup valueGroup,
Expand Down
@@ -0,0 +1,31 @@
/*
* Copyright (c) 2002-2018 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j 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, either version 3 of the License, or
* (at your option) any later version.
*
* 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, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.impl.newapi;

import org.neo4j.internal.kernel.api.NodeIndexTransactionStateTestBase;

public class NodeIndexTransactionStateTest extends NodeIndexTransactionStateTestBase<WriteTestSupport>
{
@Override
public WriteTestSupport newTestSupport()
{
return new WriteTestSupport();
}
}

0 comments on commit 67e8219

Please sign in to comment.