Find only supports one Column to filter on.
The parser for this will be the same for both Find/Filter with the only different is the Filter supports multiple columns.
In trying to keep this patch small, only WSTR/BSTR/Integer have been supported. (The Common cases).
From: Alistair Leslie-Hughes leslie_alistair@hotmail.com
Find only supports one Column to filter on.
The parser for this will be the same for both Find/Filter with the only different is the Filter supports multiple columns.
In trying to keep this patch small, only WSTR/BSTR/Integer have been supported. (The Common cases). --- dlls/msado15/Makefile.in | 6 + dlls/msado15/filter.l | 101 ++++++++++++++ dlls/msado15/filter.y | 134 ++++++++++++++++++ dlls/msado15/msado15_private.h | 69 +++++++++ dlls/msado15/recordset.c | 246 ++++++++++++++++++++++++++++----- dlls/msado15/tests/msado15.c | 19 ++- 6 files changed, 542 insertions(+), 33 deletions(-) create mode 100644 dlls/msado15/filter.l create mode 100644 dlls/msado15/filter.y
diff --git a/dlls/msado15/Makefile.in b/dlls/msado15/Makefile.in index 9c1fba502c5..81847e997bc 100644 --- a/dlls/msado15/Makefile.in +++ b/dlls/msado15/Makefile.in @@ -8,6 +8,12 @@ C_SRCS = \ recordset.c \ stream.c
+LEX_SRCS = \ + filter.l \ + +BISON_SRCS = \ + filter.y + IDL_SRCS = \ msado15_classes.idl \ msado15_tlb.idl diff --git a/dlls/msado15/filter.l b/dlls/msado15/filter.l new file mode 100644 index 00000000000..6b12a7decca --- /dev/null +++ b/dlls/msado15/filter.l @@ -0,0 +1,101 @@ +/* + * Copyright 2021 Alistair Leslie-Hughes + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ +%option noyywrap +%option caseless +/* %option prefix="filter_" */ +%option noinput nounput never-interactive +%option reentrant bison-bridge + +%{ +#include <stdio.h> + +#include "msado15_private.h" +#include "filter.tab.h" + +#include "wine/debug.h" + +WINE_DEFAULT_DEBUG_CHANNEL(msado15); + +enum joined +{ + join_none, + join_and, + join_or +}; + +%} + +%% + +[ \t] { /* ignore all whitespace */ } ; +[0-9]+.[0-9]+ { yylval->fval = atof(yytext); return T_FLOAT; } +[0-9]+ { yylval->ival = atoi(yytext); return T_INT; } +['#][^'].*['#] { + yylval->sval = malloc(strlen(yytext)-1); + memcpy(yylval->sval, yytext+1, strlen(yytext)-2); + yylval->sval[strlen(yytext)-2] = '\0'; + return T_STRING; + } + +\n { return T_NEWLINE;} +"(" { /* Ignored for now */ } +")" { /* Ignored for now */ } + +"=" { yylval->ival = op_equal; return TOKEN_EQ; } +">" { yylval->ival = op_greater; return TOKEN_GREATER; } +"<" { yylval->ival = op_less; return TOKEN_LESS; } +"<>" { yylval->ival = op_not_equal; return TOKEN_NOT_EQ; } +">=" { yylval->ival = op_greater_equal; return TOKEN_GREATER_EQ; } +"<=" { yylval->ival = op_less_equal; return TOKEN_LESS_EQ; } +"like" { yylval->ival = op_like; return TOKEN_LIKE; } + +"and" { yylval->ival = join_and; return TOKEN_AND; } +"or" { yylval->ival = join_or; return TOKEN_OR; } + + +[A-Za-z_0-9.]* { yylval->token = strdup(yytext); return TOKEN_COLUMN; } + +%% + +BOOL recordset_parse_filter(WCHAR *filter, struct recordset *recordset) +{ + char *str; + parser_param p; + YY_BUFFER_STATE buffer; + int len; + + TRACE("(%s)\n", debugstr_w(filter)); + memset(&p, 0, sizeof(parser_param)); + p.recordset = recordset; + + yylex_init(&p.yyscanner); + yyset_extra(&p, p.yyscanner); + + len = WideCharToMultiByte(CP_ACP, 0, filter, wcslen(filter)+1, 0, 0, NULL, NULL); + str = malloc(len); + + len = WideCharToMultiByte(CP_ACP, 0, filter, wcslen(filter)+1, str, len, NULL, NULL); + + buffer = yy_scan_string(str, p.yyscanner); + yyparse(&p, p.yyscanner); + yy_delete_buffer(buffer, p.yyscanner); + + free(str); + + return TRUE; +} \ No newline at end of file diff --git a/dlls/msado15/filter.y b/dlls/msado15/filter.y new file mode 100644 index 00000000000..15760ebd29f --- /dev/null +++ b/dlls/msado15/filter.y @@ -0,0 +1,134 @@ +/* + * Copyright 2021 Alistair Leslie-Hughes + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ +%{ + +#include <stdio.h> +#include <stdlib.h> + +#define COBJMACROS +#include "objbase.h" +#include "msado15_backcompat.h" + +#include "oledb.h" +#include "sqlucode.h" + +#include "msado15_private.h" + +#include "wine/debug.h" + +WINE_DEFAULT_DEBUG_CHANNEL(msado15); + +void yyerror(parser_param* param, void const* scanner, char const* msg); +parser_param* yyget_extra ( void* yyscanner ); +int yylex(void *yylval, void *scanner); +%} + +%define api.pure +%parse-param {parser_param* p} +%parse-param {void* scanner} +%lex-param {yyscan_t* scanner} + +%union { + char *token; + int operation; + int join; + + /* Possible values types */ + int ival; + float fval; + char *sval; +} + +%token<ival> T_INT +%token<fval> T_FLOAT +%token<sval> T_STRING + +%token T_NEWLINE + +%type<operation> oparations + +%token<operation> TOKEN_GREATER +%token<operation> TOKEN_GREATER_EQ +%token<operation> TOKEN_LESS +%token<operation> TOKEN_LESS_EQ +%token<operation> TOKEN_NOT_EQ +%token<operation> TOKEN_EQ +%token<operation> TOKEN_LIKE + +%token TOKEN_AND +%token TOKEN_OR + +%token<token> TOKEN_COLUMN + + +%start filter + +%% + +filter: + | filter line +; + +line: T_NEWLINE + | TOKEN_COLUMN oparations T_INT { + filter_add_integer_column(yyget_extra(scanner)->recordset, $1, $2, $3); + } + | TOKEN_COLUMN oparations T_FLOAT { + filter_add_float_column(yyget_extra(scanner)->recordset, $1, $2, $3); + } + | TOKEN_COLUMN oparations T_STRING + { + filter_add_string_column(yyget_extra(scanner)->recordset, $1, $2, $3); + } + + /* Like is always followed by a string */ + | TOKEN_COLUMN TOKEN_LIKE T_STRING + { + int i = 0; + char *stripped = malloc(strlen($3) + 1); + while(*$3 != '\0') + { + if(*$3 != '*') + { + stripped[i++] = *$3; + } + $3++; + } + FIXME("Stripped: %s, %s\n", debugstr_a($3), debugstr_a(stripped)); + filter_add_string_column(yyget_extra(scanner)->recordset, $1, $2, $3); + free(stripped); + } + + | TOKEN_AND { FIXME(" AND "); } + | TOKEN_OR { FIXME(" OR "); } +; + +oparations: TOKEN_GREATER + | TOKEN_GREATER_EQ + | TOKEN_LESS + | TOKEN_LESS_EQ + | TOKEN_NOT_EQ + | TOKEN_EQ +; + +%% + +void yyerror(parser_param* param, void const* scanner, char const* msg) +{ + FIXME("%p, %p, error: %s\n", param, scanner, msg); +} \ No newline at end of file diff --git a/dlls/msado15/msado15_private.h b/dlls/msado15/msado15_private.h index f7062c94b27..3e36fb677d3 100644 --- a/dlls/msado15/msado15_private.h +++ b/dlls/msado15/msado15_private.h @@ -19,6 +19,75 @@ #ifndef _WINE_MSADO15_PRIVATE_H_ #define _WINE_MSADO15_PRIVATE_H_
+#define COBJMACROS +#include "objbase.h" +#include "msado15_backcompat.h" +#include "oledb.h" +#include "sqlucode.h" + +#include "wine/list.h" +#include "wine/debug.h" + +struct column_filter +{ + struct list entry; + + LONG column; + int condition; + VARIANT value; +}; + +struct fields; +struct recordset +{ + _Recordset Recordset_iface; + ADORecordsetConstruction ADORecordsetConstruction_iface; + ISupportErrorInfo ISupportErrorInfo_iface; + LONG refs; + LONG state; + struct fields *fields; + LONG count; + LONG allocated; + LONG index; + VARIANT *data; + CursorLocationEnum cursor_location; + CursorTypeEnum cursor_type; + IRowset *row_set; + EditModeEnum editmode; + VARIANT filter; + BSTR criteria; + + DBTYPE *columntypes; + HACCESSOR *haccessors; + + struct list filters; +}; + +typedef struct _parser_param +{ + void* yyscanner; + + struct recordset *recordset; +} parser_param; + +#define YY_EXTRA_TYPE parser_param* + +enum operations +{ + op_equal, + op_not_equal, + op_less, + op_less_equal, + op_greater, + op_greater_equal, + op_like +}; + +BOOL recordset_parse_filter(WCHAR *filter, struct recordset *recordset) DECLSPEC_HIDDEN; +void filter_add_integer_column(struct recordset *recordset, char *name, LONG cond, LONG value) DECLSPEC_HIDDEN; +void filter_add_float_column(struct recordset *recordset, char *name, LONG cond, float value) DECLSPEC_HIDDEN; +void filter_add_string_column(struct recordset *recordset, char *name, LONG cond, char *value) DECLSPEC_HIDDEN; + #define MAKE_ADO_HRESULT( err ) MAKE_HRESULT( SEVERITY_ERROR, FACILITY_CONTROL, err )
HRESULT Command_create( void ** ) DECLSPEC_HIDDEN; diff --git a/dlls/msado15/recordset.c b/dlls/msado15/recordset.c index 13807aa1c5a..9f885d9dbe0 100644 --- a/dlls/msado15/recordset.c +++ b/dlls/msado15/recordset.c @@ -20,41 +20,12 @@ #include <assert.h> #include "windef.h" #include "winbase.h" -#define COBJMACROS -#include "objbase.h" -#include "msado15_backcompat.h" -#include "oledb.h" -#include "sqlucode.h" - +#include "msado15_private.h" #include "wine/debug.h"
-#include "msado15_private.h"
WINE_DEFAULT_DEBUG_CHANNEL(msado15);
-struct fields; -struct recordset -{ - _Recordset Recordset_iface; - ADORecordsetConstruction ADORecordsetConstruction_iface; - ISupportErrorInfo ISupportErrorInfo_iface; - LONG refs; - LONG state; - struct fields *fields; - LONG count; - LONG allocated; - LONG index; - VARIANT *data; - CursorLocationEnum cursor_location; - CursorTypeEnum cursor_type; - IRowset *row_set; - EditModeEnum editmode; - VARIANT filter; - - DBTYPE *columntypes; - HACCESSOR *haccessors; -}; - struct fields { Fields Fields_iface; @@ -970,6 +941,82 @@ static HRESULT map_index( struct fields *fields, VARIANT *index, ULONG *ret ) return MAKE_ADO_HRESULT(adErrItemNotFound); }
+static inline WCHAR *heap_strdupAtoW(const char *str) +{ + LPWSTR ret = NULL; + + if(str) { + DWORD len; + + len = MultiByteToWideChar(CP_ACP, 0, str, -1, NULL, 0); + ret = malloc(len*sizeof(WCHAR)); + if(ret) + MultiByteToWideChar(CP_ACP, 0, str, -1, ret, len); + } + + return ret; +} + +static LONG find_column_index(struct fields *fields, char *name) +{ + VARIANT column; + LONG ret = -1; + ULONG index; + WCHAR *str = heap_strdupAtoW(name); + + V_VT(&column) = VT_BSTR; + V_BSTR(&column) = SysAllocString(str); + + free(str); + + if (map_index(fields, &column, &index) == S_OK) + ret = index; + VariantClear(&column); + + return ret; +} + +void filter_add_integer_column(struct recordset *recordset, char *name, LONG cond, LONG value) +{ + struct column_filter *filter; + + filter = calloc(1, sizeof(*filter)); + filter->column = find_column_index(recordset->fields, name); + filter->condition = cond; + V_VT(&filter->value) = VT_I4; + V_I4(&filter->value) = value; + + list_add_head(&recordset->filters, &filter->entry); +} + +void filter_add_float_column(struct recordset *recordset, char *name, LONG cond, float value) +{ + struct column_filter *filter; + + filter = calloc(1, sizeof(*filter)); + filter->column = find_column_index(recordset->fields, name); + filter->condition = cond; + V_VT(&filter->value) = VT_I8; + V_I8(&filter->value) = value; + + list_add_head(&recordset->filters, &filter->entry); +} + +void filter_add_string_column(struct recordset *recordset, char *name, LONG cond, char *value) +{ + struct column_filter *filter; + WCHAR *str = heap_strdupAtoW(value); + + filter = calloc(1, sizeof(*filter)); + filter->column = find_column_index(recordset->fields, name); + filter->condition = cond; + V_VT(&filter->value) = VT_BSTR; + V_BSTR(&filter->value) = SysAllocString(str); + free(str); + + list_add_head(&recordset->filters, &filter->entry); +} + static HRESULT WINAPI fields_get_Item( Fields *iface, VARIANT index, Field **obj ) { struct fields *fields = impl_from_Fields( iface ); @@ -1183,6 +1230,20 @@ static HRESULT fields_create( struct recordset *recordset, struct fields **ret ) return S_OK; }
+static void clear_recordset_filter(struct recordset *recordset) +{ + struct column_filter *filters, *filters2; + + LIST_FOR_EACH_ENTRY_SAFE(filters, filters2, &recordset->filters, struct column_filter, entry) + { + list_remove(&filters->entry); + VariantClear(&filters->value); + free(filters); + } + SysFreeString(recordset->criteria); + recordset->criteria = NULL; +} + static inline struct recordset *impl_from_Recordset( _Recordset *iface ) { return CONTAINING_RECORD( iface, struct recordset, Recordset_iface ); @@ -1207,6 +1268,8 @@ static void close_recordset( struct recordset *recordset ) ULONG i; IAccessor *accessor;
+ clear_recordset_filter(recordset); + if (recordset->haccessors) IRowset_QueryInterface(recordset->row_set, &IID_IAccessor, (void**)&accessor);
@@ -2301,12 +2364,129 @@ static HRESULT WINAPI recordset_put_MarshalOptions( _Recordset *iface, MarshalOp return E_NOTIMPL; }
+static BOOL column_match_string(int condition, BSTR col_data, BSTR filter_data) +{ + BOOL match = FALSE; + + switch (condition) + { + case op_equal: + match = lstrcmpW(col_data, filter_data) == 0; + break; + case op_not_equal: + match = lstrcmpW(col_data, filter_data) != 0; + break; + case op_like: + FIXME("LIKE currently not supported\n"); + break; + default: + FIXME("Unsupported condition %d\n", condition); + } + + return match; +} + +static BOOL column_match_integer(int condition, INT col_data, INT filter_data) +{ + BOOL match = FALSE; + + switch (condition) + { + case op_equal: + match = col_data == filter_data; + break; + case op_not_equal: + match = col_data != filter_data; + break; + case op_less: + match = col_data < filter_data; + break; + case op_less_equal: + match = col_data <= filter_data; + break; + case op_greater: + match = col_data > filter_data; + break; + case op_greater_equal: + match = col_data >= filter_data; + break; + default: + FIXME("Unsupported condition %d\n", condition); + } + + return match; +} + static HRESULT WINAPI recordset_Find( _Recordset *iface, BSTR criteria, LONG skip_records, SearchDirectionEnum search_direction, VARIANT start ) { - FIXME( "%p, %s, %ld, %d, %s\n", iface, debugstr_w(criteria), skip_records, search_direction, + struct recordset *recordset = impl_from_Recordset( iface ); + struct column_filter *filters; + DataTypeEnum datatype; + BOOL found = FALSE; + ULONG colcnt; + + TRACE( "%p, %s, %ld, %d, %s\n", recordset, debugstr_w(criteria), skip_records, search_direction, debugstr_variant(&start) ); - return E_NOTIMPL; + + if (recordset->state != adStateOpen) return MAKE_ADO_HRESULT( adErrObjectClosed ); + + if (!recordset->criteria || wcscmp(recordset->criteria, criteria) != 0) + { + clear_recordset_filter(recordset); + + recordset->criteria = SysAllocString(criteria); + + /* Parse criteria */ + recordset_parse_filter(criteria, recordset); + } + + if (list_count(&recordset->filters) != 1) + { + WARN("Invalid number of filters.\n"); + clear_recordset_filter(recordset); + return MAKE_ADO_HRESULT( adErrDataConversion ); + } + + colcnt = get_column_count(recordset); + + filters = LIST_ENTRY(list_head(&recordset->filters), struct column_filter, entry); + field_get_Type(recordset->fields->field[filters->column], &datatype); + + recordset->index += skip_records; + for(; recordset->index < recordset->count; recordset->index++) + { + found = FALSE; + + switch(datatype) + { + case adBSTR: + case adWChar: + { + found = column_match_string(filters->condition, + V_BSTR(&recordset->data[recordset->index * colcnt + filters->column]), + V_BSTR(&filters->value)); + break; + } + case adInteger: + { + found = column_match_integer(filters->condition, + V_I4(&recordset->data[recordset->index * colcnt + filters->column]), + V_I4(&filters->value)); + break; + } + default: + FIXME("Unsupported datatype %d\n", datatype); + } + + if (found) + { + TRACE("Return record %ld\n", recordset->index); + break; + } + } + + return S_OK; }
static HRESULT WINAPI recordset_Cancel( _Recordset *iface ) @@ -2713,6 +2893,8 @@ HRESULT Recordset_create( void **obj ) VariantInit( &recordset->filter ); recordset->columntypes = NULL; recordset->haccessors = NULL; + recordset->criteria = NULL; + list_init(&recordset->filters);
*obj = &recordset->Recordset_iface; TRACE( "returning iface %p\n", *obj ); diff --git a/dlls/msado15/tests/msado15.c b/dlls/msado15/tests/msado15.c index 724787870e0..8016474c8f2 100644 --- a/dlls/msado15/tests/msado15.c +++ b/dlls/msado15/tests/msado15.c @@ -57,7 +57,7 @@ static void test_Recordset(void) VARIANT missing, val, index; CursorLocationEnum location; CursorTypeEnum cursor; - BSTR name; + BSTR name, crit; HRESULT hr; VARIANT bookmark, filter; EditModeEnum editmode; @@ -143,6 +143,12 @@ static void test_Recordset(void) hr = _Recordset_put_Filter( recordset, filter ); ok( hr == S_OK, "got %08lx\n", hr );
+ crit = SysAllocString(L"colu1=1"); + V_VT( &index ) = VT_EMPTY; + hr = _Recordset_Find( recordset, crit, 0, adSearchForward, index ); + ok( hr == MAKE_ADO_HRESULT( adErrObjectClosed ), "got %08lx\n", hr ); + SysFreeString(crit); + VariantInit( &missing ); hr = _Recordset_AddNew( recordset, missing, missing ); ok( hr == MAKE_ADO_HRESULT( adErrObjectClosed ), "got %08lx\n", hr ); @@ -229,6 +235,17 @@ static void test_Recordset(void) hr = _Recordset_Open( recordset, missing, missing, adOpenStatic, adLockBatchOptimistic, adCmdUnspecified ); ok( hr == MAKE_ADO_HRESULT( adErrObjectOpen ), "got %08lx\n", hr );
+ V_VT( &index ) = VT_EMPTY; + crit = SysAllocString(L"field = 1 OR field = 2"); + hr = _Recordset_Find(recordset, crit, 0, adSearchForward, index); + ok( hr == MAKE_ADO_HRESULT( adErrDataConversion ), "got %08lx\n", hr ); + SysFreeString(crit); + + crit = SysAllocString(L""); + hr = _Recordset_Find(recordset, crit, 0, adSearchForward, index); + ok( hr == MAKE_ADO_HRESULT( adErrDataConversion ), "got %08lx\n", hr ); + SysFreeString(crit); + state = -1; hr = _Recordset_get_State( recordset, &state ); ok( hr == S_OK, "got %08lx\n", hr );
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.l:
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
- This library 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
- Lesser General Public License for more details.
- You should have received a copy of the GNU Lesser General Public
- License along with this library; if not, write to the Free Software
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
- */
+%option noyywrap +%option caseless +/* %option prefix="filter_" */
Why is this here but commented out?
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.l:
}+\n { return T_NEWLINE;} +"(" { /* Ignored for now */ } +")" { /* Ignored for now */ }
+"=" { yylval->ival = op_equal; return TOKEN_EQ; } +">" { yylval->ival = op_greater; return TOKEN_GREATER; } +"<" { yylval->ival = op_less; return TOKEN_LESS; } +"<>" { yylval->ival = op_not_equal; return TOKEN_NOT_EQ; } +">=" { yylval->ival = op_greater_equal; return TOKEN_GREATER_EQ; } +"<=" { yylval->ival = op_less_equal; return TOKEN_LESS_EQ; } +"like" { yylval->ival = op_like; return TOKEN_LIKE; }
+"and" { yylval->ival = join_and; return TOKEN_AND; } +"or" { yylval->ival = join_or; return TOKEN_OR; }
The ival seems redundant with the token value; you should only need the latter.
The quotes around simple string tokens seem a little redundant.
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.l:
+"(" { /* Ignored for now */ } +")" { /* Ignored for now */ }
+"=" { yylval->ival = op_equal; return TOKEN_EQ; } +">" { yylval->ival = op_greater; return TOKEN_GREATER; } +"<" { yylval->ival = op_less; return TOKEN_LESS; } +"<>" { yylval->ival = op_not_equal; return TOKEN_NOT_EQ; } +">=" { yylval->ival = op_greater_equal; return TOKEN_GREATER_EQ; } +"<=" { yylval->ival = op_less_equal; return TOKEN_LESS_EQ; } +"like" { yylval->ival = op_like; return TOKEN_LIKE; }
+"and" { yylval->ival = join_and; return TOKEN_AND; } +"or" { yylval->ival = join_or; return TOKEN_OR; }
+[A-Za-z_0-9.]* { yylval->token = strdup(yytext); return TOKEN_COLUMN; }
This is a bit suspicious; is this really the column rules? Note that this will match things like "..." or "0a". I don't know SQL syntax, though, so maybe this is correct...
The lack of tests is a bit concerning.
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.l:
+"=" { yylval->ival = op_equal; return TOKEN_EQ; } +">" { yylval->ival = op_greater; return TOKEN_GREATER; } +"<" { yylval->ival = op_less; return TOKEN_LESS; } +"<>" { yylval->ival = op_not_equal; return TOKEN_NOT_EQ; } +">=" { yylval->ival = op_greater_equal; return TOKEN_GREATER_EQ; } +"<=" { yylval->ival = op_less_equal; return TOKEN_LESS_EQ; } +"like" { yylval->ival = op_like; return TOKEN_LIKE; }
+"and" { yylval->ival = join_and; return TOKEN_AND; } +"or" { yylval->ival = join_or; return TOKEN_OR; }
+[A-Za-z_0-9.]* { yylval->token = strdup(yytext); return TOKEN_COLUMN; }
+%%
In general you want a rule like
. {return yytext[0];}
This does two things:
- in the absence of such a rule, the lexer will silently accept any characters it cannot match, which probably isn't what you want (you probably want them to generate a syntax error instead, which is easily achieved by simply providing no corresponding parser rules for the token)
- it allows you to express single-character tokens (e.g. '=') as a literal character in the parser, and at the same time removes the need to specify them in the lexer.
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.l:
- int len;
- TRACE("(%s)\n", debugstr_w(filter));
- memset(&p, 0, sizeof(parser_param));
- p.recordset = recordset;
- yylex_init(&p.yyscanner);
- yyset_extra(&p, p.yyscanner);
- len = WideCharToMultiByte(CP_ACP, 0, filter, wcslen(filter)+1, 0, 0, NULL, NULL);
- str = malloc(len);
- len = WideCharToMultiByte(CP_ACP, 0, filter, wcslen(filter)+1, str, len, NULL, NULL);
- buffer = yy_scan_string(str, p.yyscanner);
- yyparse(&p, p.yyscanner);
You probably want to check the return value, which is 1 in the case of a parse error and 2 in an out-of-memory condition. In some lexers I've written parse errors are handled specially, but that's not done here yet at least, and you probably want to catch the out-of-memory condition anyway.
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.l:
+%%
+BOOL recordset_parse_filter(WCHAR *filter, struct recordset *recordset) +{
- char *str;
- parser_param p;
- YY_BUFFER_STATE buffer;
- int len;
- TRACE("(%s)\n", debugstr_w(filter));
- memset(&p, 0, sizeof(parser_param));
- p.recordset = recordset;
- yylex_init(&p.yyscanner);
- yyset_extra(&p, p.yyscanner);
There is also a function yylex_init_extra(), which does both in a single call.
You may consider adding a bit of type safety by specifying the type of the "extra" parameter, as follows:
%option extra-type="parser_param *"
That said, you never currently use the scanner in the lexer itself. (You do retrieve it from the lexer in the parser, but this is unnecessary; see my comments below.) I don't know if you have plans to, but if not, note that it's not actually necessary to set yyextra if you only use it in the parser.
Also, the lexer is never destroyed [i.e. yylex_destroy()].
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.l:
+#include "wine/debug.h"
+WINE_DEFAULT_DEBUG_CHANNEL(msado15);
+enum joined +{
- join_none,
- join_and,
- join_or
+};
+%}
+%%
+[ \t] { /* ignore all whitespace */ } ;
What about \r?
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.l:
+WINE_DEFAULT_DEBUG_CHANNEL(msado15);
+enum joined +{
- join_none,
- join_and,
- join_or
+};
+%}
+%%
+[ \t] { /* ignore all whitespace */ } ; +[0-9]+.[0-9]+ { yylval->fval = atof(yytext); return T_FLOAT; } +[0-9]+ { yylval->ival = atoi(yytext); return T_INT; }
Does SQL have bases to worry about? (E.g. is 0xa valid? Is 0123 an octal number?)
Is overflow a concern here?
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.l:
+WINE_DEFAULT_DEBUG_CHANNEL(msado15);
+enum joined +{
- join_none,
- join_and,
- join_or
+};
+%}
+%%
+[ \t] { /* ignore all whitespace */ } ; +[0-9]+.[0-9]+ { yylval->fval = atof(yytext); return T_FLOAT; }
I don't know SQL, so this may be correct, but in other languages float values may include things like "1.", ".1", "1e-1", "-1.0", "1.0f" (including case). As elsewhere, tests around this area are always nice.
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.l:
- join_or
+};
+%}
+%%
+[ \t] { /* ignore all whitespace */ } ; +[0-9]+.[0-9]+ { yylval->fval = atof(yytext); return T_FLOAT; } +[0-9]+ { yylval->ival = atoi(yytext); return T_INT; } +['#][^'].*['#] {
yylval->sval = malloc(strlen(yytext)-1);memcpy(yylval->sval, yytext+1, strlen(yytext)-2);yylval->sval[strlen(yytext)-2] = '\0';return T_STRING;}
Is the dot in this rule an error?
Assuming it is, note that this will match things like:
``` 'abc# #abc' ##abc# ```
Again, I don't know SQL, so this might be correct, but it seems surprising.
Also, is there an escape character?
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.y:
+#include "wine/debug.h"
+WINE_DEFAULT_DEBUG_CHANNEL(msado15);
+void yyerror(parser_param* param, void const* scanner, char const* msg); +parser_param* yyget_extra ( void* yyscanner ); +int yylex(void *yylval, void *scanner); +%}
+%define api.pure +%parse-param {parser_param* p} +%parse-param {void* scanner} +%lex-param {yyscan_t* scanner}
+%union {
- char *token;
Naming-wise, it seems clearer to say "column", to distinguish it from a lex token, although you could probably just as easily just use "sval" instead and get rid of this.
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.y:
+#include "msado15_private.h"
+#include "wine/debug.h"
+WINE_DEFAULT_DEBUG_CHANNEL(msado15);
+void yyerror(parser_param* param, void const* scanner, char const* msg); +parser_param* yyget_extra ( void* yyscanner ); +int yylex(void *yylval, void *scanner); +%}
+%define api.pure +%parse-param {parser_param* p} +%parse-param {void* scanner} +%lex-param {yyscan_t* scanner}
I believe this should be "yyscan_t", no pointer.
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.y:
+%token<operation> TOKEN_EQ +%token<operation> TOKEN_LIKE
+%token TOKEN_AND +%token TOKEN_OR
+%token<token> TOKEN_COLUMN
+%start filter
+%%
+filter:
- | filter line
+;
It may be a matter of taste, but for clarity, I always recommend using %empty.
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.y:
- }
- /* Like is always followed by a string */
- | TOKEN_COLUMN TOKEN_LIKE T_STRING
- {
int i = 0;char *stripped = malloc(strlen($3) + 1);while(*$3 != '\0'){if(*$3 != '*'){stripped[i++] = *$3;}$3++;}FIXME("Stripped: %s, %s\n", debugstr_a($3), debugstr_a(stripped));
If you're just going to print a FIXME and throw this away, this doesn't seem particularly like it belongs in this patch.
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.y:
+filter:
- | filter line
+;
+line: T_NEWLINE
- | TOKEN_COLUMN oparations T_INT {
filter_add_integer_column(yyget_extra(scanner)->recordset, $1, $2, $3);- }
- | TOKEN_COLUMN oparations T_FLOAT {
filter_add_float_column(yyget_extra(scanner)->recordset, $1, $2, $3);- }
- | TOKEN_COLUMN oparations T_STRING
- {
filter_add_string_column(yyget_extra(scanner)->recordset, $1, $2, $3);- }
The brace placement here is inconsistent.
The "parser_param" structure is passed as a parameter to yyparse, so you don't need to retrieve it from the scanner here. You can access it directly as "p", as specified in the %parse-param directive.
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.y:
char *stripped = malloc(strlen($3) + 1);while(*$3 != '\0'){if(*$3 != '*'){stripped[i++] = *$3;}$3++;}FIXME("Stripped: %s, %s\n", debugstr_a($3), debugstr_a(stripped));filter_add_string_column(yyget_extra(scanner)->recordset, $1, $2, $3);free(stripped);- }
- | TOKEN_AND { FIXME(" AND "); }
- | TOKEN_OR { FIXME(" OR "); }
If you're not handling them I'd advise just to leave these out, especially since I suspect that the rules themselves aren't actually right.
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.y:
if(*$3 != '*'){stripped[i++] = *$3;}$3++;}FIXME("Stripped: %s, %s\n", debugstr_a($3), debugstr_a(stripped));filter_add_string_column(yyget_extra(scanner)->recordset, $1, $2, $3);free(stripped);- }
- | TOKEN_AND { FIXME(" AND "); }
- | TOKEN_OR { FIXME(" OR "); }
+;
+oparations: TOKEN_GREATER
Spelling error, "operations".
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.y:
- | TOKEN_AND { FIXME(" AND "); }
- | TOKEN_OR { FIXME(" OR "); }
+;
+oparations: TOKEN_GREATER
- | TOKEN_GREATER_EQ
- | TOKEN_LESS
- | TOKEN_LESS_EQ
- | TOKEN_NOT_EQ
- | TOKEN_EQ
+;
+%%
+void yyerror(parser_param* param, void const* scanner, char const* msg)
It should be possible to make yyerror() static.
Zebediah Figura (@zfigura) commented about dlls/msado15/tests/msado15.c:
hr = _Recordset_Open( recordset, missing, missing, adOpenStatic, adLockBatchOptimistic, adCmdUnspecified ); ok( hr == MAKE_ADO_HRESULT( adErrObjectOpen ), "got %08lx\n", hr );
- V_VT( &index ) = VT_EMPTY;
- crit = SysAllocString(L"field = 1 OR field = 2");
- hr = _Recordset_Find(recordset, crit, 0, adSearchForward, index);
- ok( hr == MAKE_ADO_HRESULT( adErrDataConversion ), "got %08lx\n", hr );
- SysFreeString(crit);
- crit = SysAllocString(L"");
- hr = _Recordset_Find(recordset, crit, 0, adSearchForward, index);
- ok( hr == MAKE_ADO_HRESULT( adErrDataConversion ), "got %08lx\n", hr );
- SysFreeString(crit);
The lack of any tests for successful matches, or successful non-matches, strikes me as a bit concerning.
Zebediah Figura (@zfigura) commented about dlls/msado15/filter.y:
+#define COBJMACROS +#include "objbase.h" +#include "msado15_backcompat.h"
+#include "oledb.h" +#include "sqlucode.h"
+#include "msado15_private.h"
+#include "wine/debug.h"
+WINE_DEFAULT_DEBUG_CHANNEL(msado15);
+void yyerror(parser_param* param, void const* scanner, char const* msg); +parser_param* yyget_extra ( void* yyscanner ); +int yylex(void *yylval, void *scanner);
In vkd3d-shader, we use %code provides for the declaration of yylex(), to ensure that it agrees with the definition that flex generates. You may consider doing that as well.
Zebediah Figura (@zfigura) commented about dlls/msado15/Makefile.in:
recordset.c \ stream.c
+LEX_SRCS = \
- filter.l \
Looks like a stray backslash here.
-
Alistair Leslie-Hughes -
Alistair Leslie-Hughes (＠alesliehughes)
-
Zebediah Figura (＠zfigura)