Commit 66304427 authored by gmantele's avatar gmantele
Browse files

[ADQL] Fix nasty infinite loop when wrapping matches with SimpleReplaceHandler. 

This infinite loop occured only when the replacement object is just
a wrapping of the matching object ; after replacement, the new object was
inspected for matching objects.

Example: infinite loop if we want to wrap all foo(...) functions with
         the function ROUND in the following query:
    SELECT foo(foo(123)) FROM myTable
	     Expected result:
    SELECT ROUND(foo(ROUND(foo(123)))) FROM myTable
	     But generated result was:
    SELECT ROUND(ROUND(ROUND(......foo(foo(123))))) FROM myTable
parent 232bf9c9

src/adql/search/SimpleReplaceHandler.java

+16 −11
Original line number Diff line number Diff line
@@ -35,7 +35,7 @@ import adql.query.ADQLObject; 
 * </ul>

 * 

 * @author Gr&eacute;gory Mantelet (CDS;ARI)

 * @version 1.4 (05/2016)

 * @version 1.4 (06/2016)

 * 

 * @see RemoveHandler

 */
@@ -134,29 +134,34 @@ public abstract class SimpleReplaceHandler extends SimpleSearchHandler implement 
			addMatch(startObj, null);



		Stack<ADQLIterator> stackIt = new Stack<ADQLIterator>();

		Stack<ADQLObject> stackObj = new Stack<ADQLObject>();

		ADQLObject obj = null;

		ADQLIterator it = startObj.adqlIterator();



		while(!isFinished()){



			// Fetch the next ADQL object to test:

			do{

				if (it != null && it.hasNext())

				if (it != null && it.hasNext()){

					// Get the next object:

					obj = it.next();

				else if (!stackIt.isEmpty())

					// ...but continue the research inside it as long as it is possible:

					if (obj != null && goInto(obj)){

						stackIt.push(it);

						stackObj.push(obj);

						it = obj.adqlIterator();

						obj = null;

					}

				}else if (!stackIt.isEmpty()){

					it = stackIt.pop();

				else

					obj = stackObj.pop();

				}else

					return;

			}while(obj == null);



			// Add the current object if it is matching:

			if (match(obj))

				obj = addMatchAndReplace(obj, it);



			// Continue the research inside the current object (or the new object if a replacement has been performed):

			if (obj != null && goInto(obj)){

				stackIt.push(it);

				it = obj.adqlIterator();

			}

				addMatchAndReplace(obj, it);



			obj = null;

		}

test/adql/search/TestSimpleReplaceHandler.java

+54 −0
Original line number Diff line number Diff line
@@ -11,6 +11,7 @@ import adql.query.ADQLObject; 
import adql.query.ADQLQuery;

import adql.query.operand.function.DefaultUDF;

import adql.query.operand.function.MathFunction;

import adql.query.operand.function.MathFunctionType;



public class TestSimpleReplaceHandler {
@@ -64,4 +65,57 @@ public class TestSimpleReplaceHandler { 


	}



	@Test

	public void testWrappingReplacement(){

		/* WHY THIS TEST?

		 * 

		 * In case you just want to wrap a matched object, you replace it by the wrapping object initialized

		 * with the matched object.

		 * 

		 * In a first version, the replacement was done and then the ReplaceHandler was going inside the new object to replace

		 * other matching objects. But of course, it will find again the first matched object and will wrap it again, and so on

		 * indefinitely => "nasty" infinite loop.

		 * 

		 * So, the replacement of the matched objects should be always done after having looked inside it.

		 */



		String testQuery = "SELECT foo(bar(123)) FROM myTable";

		try{

			// Parse the query:

			ADQLQuery query = (new ADQLParser()).parseQuery(testQuery);



			// Check it is as expected, before the replacements:

			assertEquals(testQuery, query.toADQL().replaceAll("\\n", " "));



			// Create a replace handler:

			SimpleReplaceHandler replaceHandler = new SimpleReplaceHandler(){

				@Override

				protected boolean match(ADQLObject obj){

					return obj instanceof DefaultUDF && ((DefaultUDF)obj).getName().toLowerCase().matches("(foo|bar)");

				}



				@Override

				protected ADQLObject getReplacer(ADQLObject objToReplace) throws UnsupportedOperationException{

					try{

						return new MathFunction(MathFunctionType.ROUND, (DefaultUDF)objToReplace);

					}catch(Exception e){

						e.printStackTrace(System.err);

						fail("No error should have occured here since nothing is wrong in the ADQL query used for the test. See the stack trace in the console for more details.");

						return null;

					}

				}

			};



			// Apply the wrapping:

			replaceHandler.searchAndReplace(query);

			assertEquals(2, replaceHandler.getNbMatch());

			assertEquals(replaceHandler.getNbMatch(), replaceHandler.getNbReplacement());

			assertEquals("SELECT ROUND(foo(ROUND(bar(123)))) FROM myTable", query.toADQL().replaceAll("\\n", " "));



		}catch(Exception ex){

			ex.printStackTrace(System.err);

			fail("No error should have occured here since nothing is wrong in the ADQL query used for the test. See the stack trace in the console for more details.");

		}

	}



}