Skip to content

Commit

Permalink
TEIIDDES-2345: Teiid Client not validating ROWCOUNT correctly
Browse files Browse the repository at this point in the history 
* UpdateProcedureResolver
 * Only ProcedureCommands currently having their external contexts being
   updated with the ROWCOUNT function. TriggerActions require this too

* Adds test for specific ROWCOUNT issue
  • Loading branch information
@blafond
Paul Richardson authored and blafond committed Dec 16, 2014
1 parent 9bd8792 commit e2c45d3
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 71 deletions.
107 changes: 45 additions & 62 deletions ...teiid.runtime.client/engine/org/teiid/query/resolver/command/UpdateProcedureResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import org.teiid.designer.query.sql.lang.ICommand;
import org.teiid.designer.query.sql.lang.ISPParameter;
import org.teiid.designer.query.sql.proc.ICreateProcedureCommand;
import org.teiid.designer.runtime.version.spi.TeiidServerVersion;
import org.teiid.designer.runtime.version.spi.TeiidServerVersion.Version;
import org.teiid.language.SQLConstants;
import org.teiid.language.SQLConstants.NonReserved;
Expand Down Expand Up @@ -122,13 +121,54 @@ public UpdateProcedureResolver(QueryResolver queryResolver) {
/**
* @return the dataTypeManager
*/
@Override
public DataTypeManagerService getDataTypeManager() {
if (dataTypeManager == null)
dataTypeManager = DataTypeManagerService.getInstance(getTeiidVersion());

return this.dataTypeManager;
}

/**
* @param command
* @param metadata
*/
private void addRowCountToContext(Command command, TempMetadataAdapter metadata) throws Exception {
//by creating a new group context here it means that variables will resolve with a higher precedence than input/changing
GroupContext externalGroups = command.getExternalGroupContexts();

List<ElementSymbol> symbols = new LinkedList<ElementSymbol>();

//
// Only applicable for teiid 7
//
if (command instanceof CreateUpdateProcedureCommand && ((CreateUpdateProcedureCommand)command).isUpdateProcedure()) {
resolveVirtualGroupElements((CreateUpdateProcedureCommand) command, metadata);

//add the default variables
String countVar = ProcedureReservedWords.VARIABLES + Symbol.SEPARATOR + ProcedureReservedWords.ROWS_UPDATED;
ElementSymbol updateCount = create(ASTNodes.ELEMENT_SYMBOL);
updateCount.setName(countVar);
updateCount.setType(DataTypeManagerService.DefaultDataTypes.INTEGER.getTypeClass());
symbols.add(updateCount);
}
//
// End of Only applicable to teiid 7
//

String countVar = ProcedureReservedWords.VARIABLES + Symbol.SEPARATOR + ProcedureReservedWords.ROWCOUNT;
ElementSymbol updateCount = create(ASTNodes.ELEMENT_SYMBOL);
updateCount.setName(countVar);
updateCount.setType(DataTypeManagerService.DefaultDataTypes.INTEGER.getTypeClass());
symbols.add(updateCount);

ProcedureContainerResolver.addScalarGroup(getTeiidParser(),
ProcedureReservedWords.VARIABLES,
metadata.getMetadataStore(),
externalGroups,
symbols);
}

private void resolveCommand(TriggerAction ta, TempMetadataAdapter metadata, boolean resolveNullLiterals) throws Exception {

ICreateProcedureCommand<Block, GroupSymbol, Expression, LanguageVisitor> cmd;
Expand Down Expand Up @@ -184,75 +224,18 @@ private void resolveVirtualGroupElements(CreateUpdateProcedureCommand procComman
procCommand.setSymbolMap(symbolMap);
}

@Removed(Version.TEIID_8_0)
@Deprecated
private void resolveCommand(CreateUpdateProcedureCommand procCommand, TempMetadataAdapter metadata, boolean resolveNullLiterals)
throws Exception {

//by creating a new group context here it means that variables will resolve with a higher precedence than input/changing
GroupContext externalGroups = procCommand.getExternalGroupContexts();

List<ElementSymbol> symbols = new LinkedList<ElementSymbol>();

// virtual group elements in HAS and TRANSLATE criteria have to be resolved
if (procCommand.isUpdateProcedure()) {
resolveVirtualGroupElements(procCommand, metadata);

//add the default variables
String countVar = ProcedureReservedWords.VARIABLES + Symbol.SEPARATOR + ProcedureReservedWords.ROWS_UPDATED;
ElementSymbol updateCount = create(ASTNodes.ELEMENT_SYMBOL);
updateCount.setName(countVar);
updateCount.setType(DataTypeManagerService.DefaultDataTypes.INTEGER.getTypeClass());
symbols.add(updateCount);
}

String countVar = ProcedureReservedWords.VARIABLES + Symbol.SEPARATOR + ProcedureReservedWords.ROWCOUNT;
ElementSymbol updateCount = create(ASTNodes.ELEMENT_SYMBOL);
updateCount.setName(countVar);
updateCount.setType(DataTypeManagerService.DefaultDataTypes.INTEGER.getTypeClass());
symbols.add(updateCount);

ProcedureContainerResolver.addScalarGroup(getTeiidParser(),
ProcedureReservedWords.VARIABLES,
metadata.getMetadataStore(),
externalGroups,
symbols);
resolveBlock(procCommand, procCommand.getBlock(), externalGroups, metadata);
}

private void resolveCommand(CreateProcedureCommand command, TempMetadataAdapter metadata, boolean resolveNullLiterals)
throws Exception {

//by creating a new group context here it means that variables will resolve with a higher precedence than input/changing
GroupContext externalGroups = command.getExternalGroupContexts();

List<ElementSymbol> symbols = new LinkedList<ElementSymbol>();

String countVar = ProcedureReservedWords.VARIABLES + Symbol.SEPARATOR + ProcedureReservedWords.ROWCOUNT;
ElementSymbol updateCount = create(ASTNodes.ELEMENT_SYMBOL);
updateCount.setName(countVar);
updateCount.setType(DataTypeManagerService.DefaultDataTypes.INTEGER.getTypeClass());
symbols.add(updateCount);

ProcedureContainerResolver.addScalarGroup(getTeiidParser(),
ProcedureReservedWords.VARIABLES,
metadata.getMetadataStore(),
externalGroups,
symbols);

resolveBlock(command, command.getBlock(), externalGroups, metadata);
}

/**
* @see org.teiid.query.resolver.CommandResolver#resolveCommand(org.teiid.query.sql.lang.Command, TempMetadataAdapter, boolean)
*/
@Override
public void resolveCommand(Command command, TempMetadataAdapter metadata, boolean resolveNullLiterals) throws Exception {

addRowCountToContext(command, metadata);

if (command instanceof CreateProcedureCommand)
resolveCommand((CreateProcedureCommand)command, metadata, resolveNullLiterals);
resolveBlock((CreateProcedureCommand) command, ((CreateProcedureCommand) command).getBlock(), command.getExternalGroupContexts(), metadata);
else if (command instanceof CreateUpdateProcedureCommand)
resolveCommand((CreateUpdateProcedureCommand)command, metadata, resolveNullLiterals);
resolveBlock((CreateUpdateProcedureCommand) command, ((CreateUpdateProcedureCommand) command).getBlock(), command.getExternalGroupContexts(), metadata);
else if (command instanceof TriggerAction)
resolveCommand((TriggerAction)command, metadata, resolveNullLiterals);
else
Expand Down
16 changes: 16 additions & 0 deletions ...rg.teiid.runtime.client.test/src/org/teiid/query/resolver/v8/Test8ProcedureResolving.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
@SuppressWarnings( {"javadoc", "nls"} )
public class Test8ProcedureResolving extends AbstractTestProcedureResolving {

private static final String NEW_LINE = "\n";

private Test8Factory factory;

protected Test8ProcedureResolving(ITeiidServerVersion teiidVersion) {
Expand Down Expand Up @@ -141,6 +143,20 @@ public void testProcedureCreate() throws Exception {
helpResolveUpdateProcedure(proc.toString(), userUpdateStr, Table.TriggerEvent.UPDATE);
}

@Test
public void testTEIDDES2345() throws Exception {
String proc = "FOR EACH ROW" + NEW_LINE +
"BEGIN ATOMIC" + NEW_LINE +
"DECLARE integer VARIABLES.ROWS_UPDATED;" + NEW_LINE +
"UPDATE vm1.g1 SET e1='x';" + NEW_LINE +
"VARIABLES.ROWS_UPDATED = VARIABLES.ROWCOUNT;" + NEW_LINE +
"END";

String userUpdateStr = "UPDATE vm1.g1 SET e1='x1'"; //$NON-NLS-1$

helpResolveUpdateProcedure(proc, userUpdateStr, Table.TriggerEvent.UPDATE);
}

/**
* it is not ok to redefine the loopCursor
*/
Expand Down
27 changes: 18 additions & 9 deletions tests/org.teiid.runtime.client.test/src/org/teiid/query/sql/AbstractTestQueryParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,13 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.io.UnsupportedEncodingException;
import java.math.BigInteger;
import java.sql.Date;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import org.junit.Test;
import org.teiid.core.types.DataTypeManagerService;
import org.teiid.designer.query.sql.lang.ISPParameter.ParameterInfo;
Expand All @@ -30,12 +28,12 @@
import org.teiid.query.parser.TeiidNodeFactory.ASTNodes;
import org.teiid.query.sql.lang.ArrayTable;
import org.teiid.query.sql.lang.BetweenCriteria;
import org.teiid.query.sql.lang.CacheHint;
import org.teiid.query.sql.lang.Command;
import org.teiid.query.sql.lang.CompareCriteria;
import org.teiid.query.sql.lang.CompoundCriteria;
import org.teiid.query.sql.lang.Criteria;
import org.teiid.query.sql.lang.CriteriaOperator.Operator;
import org.teiid.query.sql.lang.CacheHint;
import org.teiid.query.sql.lang.Delete;
import org.teiid.query.sql.lang.DynamicCommand;
import org.teiid.query.sql.lang.ExistsCriteria;
Expand Down Expand Up @@ -113,23 +111,34 @@ public AbstractTestQueryParser(ITeiidServerVersion teiidVersion) {
super(teiidVersion);
}

protected void helpTest(String sql, String expectedString, Command expectedCommand, boolean designerCommand) {
helpTest(sql, expectedString, expectedCommand, new ParseInfo(), true);
}

protected void helpTest(String sql, String expectedString, Command expectedCommand) {
helpTest(sql, expectedString, expectedCommand, new ParseInfo());
helpTest(sql, expectedString, expectedCommand, new ParseInfo(), false);
}

protected void helpTest(String sql, String expectedString, Command expectedCommand, ParseInfo info) {
protected void helpTest(String sql, String expectedString, Command expectedCommand, ParseInfo info, boolean designerCommand) {
Command actualCommand = null;
String actualString = null;

try {
actualCommand = parser.parseCommand(sql, info);
if (designerCommand)
actualCommand = parser.parseDesignerCommand(sql);
else
actualCommand = parser.parseCommand(sql, info);

actualString = actualCommand.toString();
} catch (Throwable e) {
fail(e.getMessage());
}

assertEquals("Command objects do not match: ", expectedCommand, actualCommand);
assertEquals("SQL strings do not match: ", expectedString, actualString);
if (expectedCommand != null)
assertEquals("Command objects do not match: ", expectedCommand, actualCommand);

if (expectedString != null)
assertEquals("SQL strings do not match: ", expectedString, actualString);
}

protected void helpTestLiteral(Boolean expected, Class<?> expectedType, String sql, String expectedSql) {
Expand Down Expand Up @@ -1818,7 +1827,7 @@ public void testStringLiteralLikeQuotedElement1() {
Query query = getFactory().newQuery(select, from);
ParseInfo info = new ParseInfo();
info.setAnsiQuotedIdentifiers(false);
helpTest("SELECT \"g\"\".\"\"a\" from g", "SELECT 'g\".\"a' FROM g", query, info);
helpTest("SELECT \"g\"\".\"\"a\" from g", "SELECT 'g\".\"a' FROM g", query, info, false);
}

/** SELECT g.x AS "select" FROM g */
Expand Down

0 comments on commit e2c45d3

Please sign in to comment.