IIF Bug in Mono.Data.SqlExpressions/Parser.jay

[joel.reed]

The following code works on MS.NET but not mono svn head (as of 9/24/07):

  DataTable dt = new DataTable();
  dt.Columns.Add("SurveyImage", Type.GetType("System.String"),
          "IIF(LMSurvey, 'g1.gif', 'g2.gif')");


1) The attached file "Iif.testcase.diff" adds a test case for
this. OK to apply?


2) I first tried fixing this by adding this code to "BoolExpr"

Index: class/System.Data/Mono.Data.SqlExpressions/Parser.jay
===================================================================
--- class/System.Data/Mono.Data.SqlExpressions/Parser.jay (revision 86284)
+++ class/System.Data/Mono.Data.SqlExpressions/Parser.jay (working copy)
@@ -118,6 +118,10 @@
  $$ = new Negation ((IExpression)$2);
  }
  | Predicate
+ | SingleColumnValue
+ {
+ $$ = new BoolOperation(Operation.EQ, (IExpression)$1, new Literal (true));
+ }
  ;
 
 Predicate


But this broke alot of test cases (about 8). I think Parser.jay needs more surgery to fix
these test cases. For example, this line in "CalcFunction"

  SUBSTRING PAROPEN Expr COMMA NumberLiteral COMMA NumberLiteral PARCLOSE

Doesn't seem correct. Expr is defined as "BoolExpr | ArithExpr". But SUBSTRING
doesn't work on a BoolOperation or ArithmeticOperation afaik. It works now because
ArithExpr can also be a "Value". Come to think of it, shouldn't NumberLiteral actually
be ArithExpr? I'll have to try that on MS.NET...

Anyway, not wanting to make lots of changes to this file without checking in here first,
I instead made this change:

Index: class/System.Data/Mono.Data.SqlExpressions/Parser.jay
===================================================================
--- class/System.Data/Mono.Data.SqlExpressions/Parser.jay (revision 86284)
+++ class/System.Data/Mono.Data.SqlExpressions/Parser.jay (working copy)
@@ -269,11 +269,19 @@
  | VAR { $$ = AggregationFunction.Var; }
  ;
 
-CalcFunction
+IifFunction
  : IIF PAROPEN BoolExpr COMMA Expr COMMA Expr PARCLOSE
  {
  $$ = new IifFunction ((IExpression)$3, (IExpression)$5, (IExpression)$7);
  }
+ | IIF PAROPEN SingleColumnValue COMMA Expr COMMA Expr PARCLOSE
+ {
+ $$ = new IifFunction (new BoolOperation(Operation.EQ, (IExpression)$3, new Literal (true)), (IExpression)$5, (IExpression)$7);
+ }
+ ;
+
+CalcFunction
+ : IifFunction
  | SUBSTRING PAROPEN Expr COMMA NumberLiteral COMMA NumberLiteral PARCLOSE
  {
  long arg1 = (long) $5;


Which does the job - but seems rather ugly to me. Any comments? Anyone interested in
this quick fix or me taking the first route & fixing up the rest of this file?

jr

---

Index: class/System.Data/Test/Mono.Data.SqlExpressions/DataColumnExpressionTest.cs
===================================================================
--- class/System.Data/Test/Mono.Data.SqlExpressions/DataColumnExpressionTest.cs (revision 86284)
+++ class/System.Data/Test/Mono.Data.SqlExpressions/DataColumnExpressionTest.cs (working copy)
@@ -8,6 +8,21 @@
  public class DataColumnExprTest
  {
  [Test]
+ public void TestDataColumnExpr0 ()
+ {
+ DataTable table = new DataTable ();
+ table.Columns.Add ("Col_0.Value", Type.GetType ("System.Int32"));
+ table.Columns.Add ("Col_1", Type.GetType ("System.Int32"));
+ table.Columns.Add ("Result", Type.GetType ("System.Int32"), "IIF(Col_0.Value, Col_1 + 5, 0)");
+
+ DataRow row = table.NewRow ();
+ row ["Col_0.Value"] = 0;
+ row ["Col_1"] = 10;
+
+ table.Rows.Add (row);
+ Assert.AreEqual (0, (int)table.Rows[0][2], "#1");
+ }
+ [Test]
  public void TestDataColumnExpr1 ()
  {
  DataTable table = new DataTable ();

---

---

Index: class/System.Data/Mono.Data.SqlExpressions/Parser.jay
===================================================================
--- class/System.Data/Mono.Data.SqlExpressions/Parser.jay (revision 86284)
+++ class/System.Data/Mono.Data.SqlExpressions/Parser.jay (working copy)
@@ -118,6 +118,10 @@
  $$ = new Negation ((IExpression)$2);
  }
  | Predicate
+ | SingleColumnValue
+ {
+ $$ = new BoolOperation(Operation.EQ, (IExpression)$1, new Literal (true));
+ }
  ;
 
 Predicate

---

---

Index: class/System.Data/Mono.Data.SqlExpressions/Parser.jay
===================================================================
--- class/System.Data/Mono.Data.SqlExpressions/Parser.jay (revision 86284)
+++ class/System.Data/Mono.Data.SqlExpressions/Parser.jay (working copy)
@@ -269,11 +269,19 @@
  | VAR { $$ = AggregationFunction.Var; }
  ;
 
-CalcFunction
+IifFunction
  : IIF PAROPEN BoolExpr COMMA Expr COMMA Expr PARCLOSE
  {
  $$ = new IifFunction ((IExpression)$3, (IExpression)$5, (IExpression)$7);
  }
+ | IIF PAROPEN SingleColumnValue COMMA Expr COMMA Expr PARCLOSE
+ {
+ $$ = new IifFunction (new BoolOperation(Operation.EQ, (IExpression)$3, new Literal (true)), (IExpression)$5, (IExpression)$7);
+ }
+ ;
+
+CalcFunction
+ : IifFunction
  | SUBSTRING PAROPEN Expr COMMA NumberLiteral COMMA NumberLiteral PARCLOSE
  {
  long arg1 = (long) $5;

---

_______________________________________________
Mono-devel-list mailing list
Mono-devel-list@...
http://lists.ximian.com/mailman/listinfo/mono-devel-list

[A Nagappan]

Hi JR,

I suggest not to break existing test-cases, when adding new code :) If
you could fix the issues that you face, it will be really cool !!!

I too like your first approach than the second.

Thanks
Nagappan
--

--
Nagappan A <anagappan@...>
Linux Desktop Testing Project - http://ldtp.freedesktop.org
http://nagappanal.blogspot.com

Novell, Inc.
SUSE* Linux Enterprise 10
Your Linux is ready*
http://www.novell.com/linux




>>> On 9/25/2007 at 8:04 AM, in message
<20070925023445.GA14676@localdomain>, Joel
Reed <joelwreed@...> wrote:
> The following code works on MS.NET but not mono svn head (as of
9/24/07):

>
>   DataTable dt = new DataTable();
>   dt.Columns.Add("SurveyImage", Type.GetType("System.String"),
>  "IIF(LMSurvey, 'g1.gif', 'g2.gif')");
>
>
> 1) The attached file "Iif.testcase.diff" adds a test case for
> this. OK to apply?
>
>
> 2) I first tried fixing this by adding this code to "BoolExpr"
>
> Index: class/System.Data/Mono.Data.SqlExpressions/Parser.jay
> ===================================================================
> ---

class/System.Data/Mono.Data.SqlExpressions/Parser.jay (revision
86284)
> +++ class/System.Data/Mono.Data.SqlExpressions/Parser.jay (working
copy)
> @@ -118,6 +118,10 @@
>   $$ = new Negation ((IExpression)$2);
>   }
>   | Predicate
> + | SingleColumnValue
> + {
> + $$ = new BoolOperation(Operation.EQ, (IExpression)$1,
new Literal (true));
> + }
>   ;
>  
>  Predicate
>
>
> But this broke alot of test cases (about 8). I think Parser.jay needs
more
> surgery to fix
> these test cases. For example, this line in "CalcFunction"
>
>   SUBSTRING PAROPEN Expr COMMA NumberLiteral COMMA NumberLiteral
PARCLOSE
>
> Doesn't seem correct. Expr is defined as "BoolExpr | ArithExpr". But

> SUBSTRING
> doesn't work on a BoolOperation or ArithmeticOperation afaik. It
works now
> because
> ArithExpr can also be a "Value". Come to think of it, shouldn't
> NumberLiteral actually
> be ArithExpr? I'll have to try that on MS.NET...
>
> Anyway, not wanting to make lots of changes to this file without
checking in
> here first,
> I instead made this change:
>
> Index: class/System.Data/Mono.Data.SqlExpressions/Parser.jay
> ===================================================================
> ---
class/System.Data/Mono.Data.SqlExpressions/Parser.jay (revision
86284)
> +++ class/System.Data/Mono.Data.SqlExpressions/Parser.jay (working
copy)
> @@ -269,11 +269,19 @@
>   | VAR { $$ = AggregationFunction.Var; }
>   ;
>  
> -CalcFunction
> +IifFunction
>   : IIF PAROPEN BoolExpr COMMA Expr COMMA Expr PARCLOSE
>   {
>   $$ = new IifFunction ((IExpression)$3, (IExpression)$5,
(IExpression)$7);
>   }
> + | IIF PAROPEN SingleColumnValue COMMA Expr COMMA Expr PARCLOSE
> + {
> + $$ = new IifFunction (new BoolOperation(Operation.EQ,
(IExpression)$3, new
> Literal (true)), (IExpression)$5, (IExpression)$7);
> + }
> + ;
> +
> +CalcFunction
> + : IifFunction
>   | SUBSTRING PAROPEN Expr COMMA NumberLiteral COMMA NumberLiteral
PARCLOSE
>   {
>   long arg1 = (long) $5;
>
>
> Which does the job - but seems rather ugly to me. Any comments?
Anyone
> interested in
> this quick fix or me taking the first route & fixing up the rest of
this
> file?
>
> jr
_______________________________________________
Mono-devel-list mailing list
Mono-devel-list@...
http://lists.ximian.com/mailman/listinfo/mono-devel-list