/ Check-in [927b95a0]
Login

Many hyperlinks are disabled.
Use anonymous login to enable hyperlinks.

Overview
Comment:Try to improve the error messages for misformed frame specifications in window definitions.
Downloads: Tarball | ZIP archive | SQL archive
Timelines: family | ancestors | descendants | both | trunk
Files: files | file ages | folders
SHA3-256: 927b95a0812787bcb3c28d1a0ea94717dc2cc60f6d480623f0c7cbce0c546fc9
User & Date: drh 2018-07-06 17:19:20
Context
2018-07-07
17:30
Fix a problem with the handling of NULL values in the min() window function. check-in: b76f35b0 user: dan tags: trunk
2018-07-06
17:19
Try to improve the error messages for misformed frame specifications in window definitions. check-in: 927b95a0 user: drh tags: trunk
14:31
Also disallow non-constant expressions in "<expr> PRECEDING" or "<expr> FOLLOWING" clauses. check-in: a6dffecc user: dan tags: trunk
Changes
Hide Diffs Unified Diffs Ignore Whitespace Patch

Changes to src/window.c.

823
824
825
826
827
828
829















830
831
832
833
834
835
836





837
838
839










840
841







842
843
844
845
846
847
848
849
850
851
852
853
854
855
856
857
858
859
860
861
862
863

864
865

866
867
868
869
870
871
872
873



874
875
876
877
878
879
880
881
882
883
884
885
886
887
888
889

890
891
892
893
894
895
896
897
...
969
970
971
972
973
974
975
976
977
978
979
980
981
982
983
....
1532
1533
1534
1535
1536
1537
1538
1539
1540
1541
1542
1543
1544
1545
1546
1547
1548
1549
1550
    Window *pNext = p->pNextWin;
    sqlite3WindowDelete(db, p);
    p = pNext;
  }
}

/*















** Allocate and return a new Window object.
*/
Window *sqlite3WindowAlloc(
  Parse *pParse, 
  int eType,
  int eStart, Expr *pStart,
  int eEnd, Expr *pEnd





){
  Window *pWin = 0;











  /* If a frame is declared "RANGE" (not "ROWS"), then it may not use
  ** either "<expr> PRECEDING" or "<expr> FOLLOWING". Additionally, the







  ** starting boundary type may not occur earlier in the following list than
  ** the ending boundary type:
  **
  **   UNBOUNDED PRECEDING
  **   <expr> PRECEDING
  **   CURRENT ROW
  **   <expr> FOLLOWING
  **   UNBOUNDED FOLLOWING
  **
  ** The parser ensures that "UNBOUNDED PRECEDING" cannot be used as an ending
  ** boundary, and than "UNBOUNDED FOLLOWING" cannot be used as a starting
  ** frame boundary.
  */
  if( eType==TK_RANGE && (pStart || pEnd) 
   || (eStart==TK_CURRENT && eEnd==TK_PRECEDING)
   || (eStart==TK_FOLLOWING && (eEnd==TK_PRECEDING || eEnd==TK_CURRENT))
   || (0==sqlite3ExprIsConstantOrFunction(pStart, 0))
   || (0==sqlite3ExprIsConstantOrFunction(pEnd, 0))
  ){
    sqlite3ErrorMsg(pParse, "unsupported window-frame type");
  }else{
    pWin = (Window*)sqlite3DbMallocZero(pParse->db, sizeof(Window));

  }


  if( pWin ){
    assert( eType );
    pWin->eType = eType;
    pWin->eStart = eStart;
    pWin->eEnd = eEnd;
    pWin->pEnd = pEnd;
    pWin->pStart = pStart;
  }else{



    sqlite3ExprDelete(pParse->db, pEnd);
    sqlite3ExprDelete(pParse->db, pStart);
  }

  return pWin;
}

/*
** Attach window object pWin to expression p.
*/
void sqlite3WindowAttach(Parse *pParse, Expr *p, Window *pWin){
  if( p ){
    if( pWin ){
      p->pWin = pWin;
      pWin->pOwner = p;
      if( p->flags & EP_Distinct ){

        sqlite3ErrorMsg(pParse,"DISTINCT is not supported for window functions");
      }
    }
  }else{
    sqlite3WindowDelete(pParse->db, pWin);
  }
}

................................................................................

/*
** A "PRECEDING <expr>" (bEnd==0) or "FOLLOWING <expr>" (bEnd==1) has just 
** been evaluated and the result left in register reg. This function generates
** VM code to check that the value is a non-negative integer and throws
** an exception if it is not.
*/
static void windowCheckFrameValue(Parse *pParse, int reg, int bEnd){
  static const char *azErr[] = {
    "frame starting offset must be a non-negative integer",
    "frame ending offset must be a non-negative integer"
  };
  Vdbe *v = sqlite3GetVdbe(pParse);
  int regZero = sqlite3GetTempReg(pParse);
  sqlite3VdbeAddOp2(v, OP_Integer, 0, regZero);
................................................................................
  sqlite3VdbeAddOp2(v, OP_OpenDup, csrStart, pMWin->iEphCsr);
  sqlite3VdbeAddOp2(v, OP_OpenDup, csrEnd, pMWin->iEphCsr);

  /* If either regStart or regEnd are not non-negative integers, throw 
  ** an exception.  */
  if( pMWin->pStart ){
    sqlite3ExprCode(pParse, pMWin->pStart, regStart);
    windowCheckFrameValue(pParse, regStart, 0);
  }
  if( pMWin->pEnd ){
    sqlite3ExprCode(pParse, pMWin->pEnd, regEnd);
    windowCheckFrameValue(pParse, regEnd, 1);
  }

  /* If this is "ROWS <expr1> FOLLOWING AND ROWS <expr2> FOLLOWING", do:
  **
  **   if( regEnd<regStart ){
  **     // The frame always consists of 0 rows
  **     regStart = regSize;







>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
|


|
<
<
<
>
>
>
>
>



>
>
>
>
>
>
>
>
>
>

|
>
>
>
>
>
>
>













<
|

<
<

|
<
<
>


>
|
<
|
|
|
|
|
<
>
>
>
|
|
<
<
|











>
|







 







|







 







|



|







823
824
825
826
827
828
829
830
831
832
833
834
835
836
837
838
839
840
841
842
843
844
845
846
847
848



849
850
851
852
853
854
855
856
857
858
859
860
861
862
863
864
865
866
867
868
869
870
871
872
873
874
875
876
877
878
879
880
881
882
883
884
885
886
887
888

889
890


891
892


893
894
895
896
897

898
899
900
901
902

903
904
905
906
907


908
909
910
911
912
913
914
915
916
917
918
919
920
921
922
923
924
925
926
927
928
....
1000
1001
1002
1003
1004
1005
1006
1007
1008
1009
1010
1011
1012
1013
1014
....
1563
1564
1565
1566
1567
1568
1569
1570
1571
1572
1573
1574
1575
1576
1577
1578
1579
1580
1581
    Window *pNext = p->pNextWin;
    sqlite3WindowDelete(db, p);
    p = pNext;
  }
}

/*
** The argument expression is an PRECEDING or FOLLOWING offset.  The
** value should be a non-negative integer.  If the value is not a
** constant, change it to NULL.  The fact that it is then a non-negative
** integer will be caught later.  But it is important not to leave
** variable values in the expression tree.
*/
static Expr *sqlite3WindowOffsetExpr(Parse *pParse, Expr *pExpr){
  if( 0==sqlite3ExprIsConstant(pExpr) ){
    sqlite3ExprDelete(pParse->db, pExpr);
    pExpr = sqlite3ExprAlloc(pParse->db, TK_NULL, 0, 0);
  }
  return pExpr;
}

/*
** Allocate and return a new Window object describing a Window Definition.
*/
Window *sqlite3WindowAlloc(
  Parse *pParse,    /* Parsing context */



  int eType,        /* Frame type. TK_RANGE or TK_ROWS */
  int eStart,       /* Start type: CURRENT, PRECEDING, FOLLOWING, UNBOUNDED */
  Expr *pStart,     /* Start window size if TK_PRECEDING or FOLLOWING */
  int eEnd,         /* End type: CURRENT, FOLLOWING, TK_UNBOUNDED, PRECEDING */
  Expr *pEnd        /* End window size if TK_FOLLOWING or PRECEDING */
){
  Window *pWin = 0;

  /* Parser assures the following: */
  assert( eType==TK_RANGE || eType==TK_ROWS );
  assert( eStart==TK_CURRENT || eStart==TK_PRECEDING
           || eStart==TK_UNBOUNDED || eStart==TK_FOLLOWING );
  assert( eEnd==TK_CURRENT || eEnd==TK_FOLLOWING
           || eEnd==TK_UNBOUNDED || eEnd==TK_PRECEDING );
  assert( (eStart==TK_PRECEDING || eStart==TK_FOLLOWING)==(pStart!=0) );
  assert( (eEnd==TK_FOLLOWING || eEnd==TK_PRECEDING)==(pEnd!=0) );


  /* If a frame is declared "RANGE" (not "ROWS"), then it may not use
  ** either "<expr> PRECEDING" or "<expr> FOLLOWING".
  */
  if( eType==TK_RANGE && (pStart!=0 || pEnd!=0) ){
    sqlite3ErrorMsg(pParse, "RANGE must use only UNBOUNDED or CURRENT ROW");
    goto windowAllocErr;
  }

  /* Additionally, the
  ** starting boundary type may not occur earlier in the following list than
  ** the ending boundary type:
  **
  **   UNBOUNDED PRECEDING
  **   <expr> PRECEDING
  **   CURRENT ROW
  **   <expr> FOLLOWING
  **   UNBOUNDED FOLLOWING
  **
  ** The parser ensures that "UNBOUNDED PRECEDING" cannot be used as an ending
  ** boundary, and than "UNBOUNDED FOLLOWING" cannot be used as a starting
  ** frame boundary.
  */

  if( (eStart==TK_CURRENT && eEnd==TK_PRECEDING)
   || (eStart==TK_FOLLOWING && (eEnd==TK_PRECEDING || eEnd==TK_CURRENT))


  ){
    sqlite3ErrorMsg(pParse, "unsupported frame delimiter for ROWS");


    goto windowAllocErr;
  }

  pWin = (Window*)sqlite3DbMallocZero(pParse->db, sizeof(Window));
  if( pWin==0 ) goto windowAllocErr;

  pWin->eType = eType;
  pWin->eStart = eStart;
  pWin->eEnd = eEnd;
  pWin->pEnd = sqlite3WindowOffsetExpr(pParse, pEnd);
  pWin->pStart = sqlite3WindowOffsetExpr(pParse, pStart);

  return pWin;

windowAllocErr:
  sqlite3ExprDelete(pParse->db, pEnd);
  sqlite3ExprDelete(pParse->db, pStart);


  return 0;
}

/*
** Attach window object pWin to expression p.
*/
void sqlite3WindowAttach(Parse *pParse, Expr *p, Window *pWin){
  if( p ){
    if( pWin ){
      p->pWin = pWin;
      pWin->pOwner = p;
      if( p->flags & EP_Distinct ){
        sqlite3ErrorMsg(pParse,
           "DISTINCT is not supported for window functions");
      }
    }
  }else{
    sqlite3WindowDelete(pParse->db, pWin);
  }
}

................................................................................

/*
** A "PRECEDING <expr>" (bEnd==0) or "FOLLOWING <expr>" (bEnd==1) has just 
** been evaluated and the result left in register reg. This function generates
** VM code to check that the value is a non-negative integer and throws
** an exception if it is not.
*/
static void windowCheckFrameOffset(Parse *pParse, int reg, int bEnd){
  static const char *azErr[] = {
    "frame starting offset must be a non-negative integer",
    "frame ending offset must be a non-negative integer"
  };
  Vdbe *v = sqlite3GetVdbe(pParse);
  int regZero = sqlite3GetTempReg(pParse);
  sqlite3VdbeAddOp2(v, OP_Integer, 0, regZero);
................................................................................
  sqlite3VdbeAddOp2(v, OP_OpenDup, csrStart, pMWin->iEphCsr);
  sqlite3VdbeAddOp2(v, OP_OpenDup, csrEnd, pMWin->iEphCsr);

  /* If either regStart or regEnd are not non-negative integers, throw 
  ** an exception.  */
  if( pMWin->pStart ){
    sqlite3ExprCode(pParse, pMWin->pStart, regStart);
    windowCheckFrameOffset(pParse, regStart, 0);
  }
  if( pMWin->pEnd ){
    sqlite3ExprCode(pParse, pMWin->pEnd, regEnd);
    windowCheckFrameOffset(pParse, regEnd, 1);
  }

  /* If this is "ROWS <expr1> FOLLOWING AND ROWS <expr2> FOLLOWING", do:
  **
  **   if( regEnd<regStart ){
  **     // The frame always consists of 0 rows
  **     regStart = regSize;

Changes to test/window6.test.

217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
...
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268











269


} {
  1 1  2 1,2  3 1,2,3  4 2,3,4  5 3,4,5
}
do_catchsql_test 9.1 {
  WITH RECURSIVE c(x) AS (VALUES(1) UNION ALL SELECT x+1 FROM c WHERE x<5)
  SELECT x, group_concat(x) OVER (ORDER BY x RANGE 2 PRECEDING)
  FROM c;
} {1 {unsupported window-frame type}}

do_catchsql_test 9.2 {
  WITH RECURSIVE c(x) AS (VALUES(1) UNION ALL SELECT x+1 FROM c WHERE x<5)
  SELECT x, group_concat(x) OVER (ORDER BY x RANGE BETWEEN UNBOUNDED PRECEDING AND 2 FOLLOWING)
  FROM c;
} {1 {unsupported window-frame type}}

do_catchsql_test 9.3 {
  WITH RECURSIVE c(x) AS (VALUES(1) UNION ALL SELECT x+1 FROM c WHERE x<5)
  SELECT count(DISTINCT x) OVER (ORDER BY x) FROM c;
} {1 {DISTINCT is not supported for window functions}}

do_catchsql_test 9.4 {
................................................................................
} {1 {near "PRECEDING": syntax error}}

foreach {tn frame} {
  1 "BETWEEN CURRENT ROW AND 4 PRECEDING"
  2 "4 FOLLOWING"
  3 "BETWEEN 4 FOLLOWING AND CURRENT ROW"
  4 "BETWEEN 4 FOLLOWING AND 2 PRECEDING"
  5 "BETWEEN a PRECEDING AND 2 FOLLOWING"
  6 "BETWEEN 4 PRECEDING AND a FOLLOWING"
} {
  do_catchsql_test 9.7.$tn "
    WITH RECURSIVE c(x) AS (VALUES(1) UNION ALL SELECT x+1 FROM c WHERE x<5)
    SELECT count() OVER (
        ORDER BY x ROWS $frame 
    ) FROM c;
  " {1 {unsupported window-frame type}}
}

finish_test





















|





|







 







<
<






|


|
>
>
>
>
>
>
>
>
>
>
>

>
>
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
...
250
251
252
253
254
255
256


257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
} {
  1 1  2 1,2  3 1,2,3  4 2,3,4  5 3,4,5
}
do_catchsql_test 9.1 {
  WITH RECURSIVE c(x) AS (VALUES(1) UNION ALL SELECT x+1 FROM c WHERE x<5)
  SELECT x, group_concat(x) OVER (ORDER BY x RANGE 2 PRECEDING)
  FROM c;
} {1 {RANGE must use only UNBOUNDED or CURRENT ROW}}

do_catchsql_test 9.2 {
  WITH RECURSIVE c(x) AS (VALUES(1) UNION ALL SELECT x+1 FROM c WHERE x<5)
  SELECT x, group_concat(x) OVER (ORDER BY x RANGE BETWEEN UNBOUNDED PRECEDING AND 2 FOLLOWING)
  FROM c;
} {1 {RANGE must use only UNBOUNDED or CURRENT ROW}}

do_catchsql_test 9.3 {
  WITH RECURSIVE c(x) AS (VALUES(1) UNION ALL SELECT x+1 FROM c WHERE x<5)
  SELECT count(DISTINCT x) OVER (ORDER BY x) FROM c;
} {1 {DISTINCT is not supported for window functions}}

do_catchsql_test 9.4 {
................................................................................
} {1 {near "PRECEDING": syntax error}}

foreach {tn frame} {
  1 "BETWEEN CURRENT ROW AND 4 PRECEDING"
  2 "4 FOLLOWING"
  3 "BETWEEN 4 FOLLOWING AND CURRENT ROW"
  4 "BETWEEN 4 FOLLOWING AND 2 PRECEDING"


} {
  do_catchsql_test 9.7.$tn "
    WITH RECURSIVE c(x) AS (VALUES(1) UNION ALL SELECT x+1 FROM c WHERE x<5)
    SELECT count() OVER (
        ORDER BY x ROWS $frame 
    ) FROM c;
  " {1 {unsupported frame delimiter for ROWS}}
}

do_catchsql_test 9.8.1 {
  WITH RECURSIVE c(x) AS (VALUES(1) UNION ALL SELECT x+1 FROM c WHERE x<5)
  SELECT count() OVER (
      ORDER BY x ROWS BETWEEN a PRECEDING AND 2 FOLLOWING
  ) FROM c;
} {1 {frame starting offset must be a non-negative integer}}
do_catchsql_test 9.8.2 {
  WITH RECURSIVE c(x) AS (VALUES(1) UNION ALL SELECT x+1 FROM c WHERE x<5)
  SELECT count() OVER (
      ORDER BY x ROWS BETWEEN 2 PRECEDING AND a FOLLOWING
  ) FROM c;
} {1 {frame ending offset must be a non-negative integer}}


finish_test