Skip to content

Fix for malformed query leading to CLI hanging. #140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions mssqlcli/jsonrpc/contracts/queryexecutestringservice.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ def get_response(self):
if response:
decoded_response = self.decode_response(response)

if isinstance(decoded_response, QueryCompleteEvent):
if isinstance(decoded_response, QueryCompleteEvent) or \
isinstance(decoded_response, QueryExecuteErrorResponseEvent):
self.finished = True
self.json_rpc_client.request_finished(self.id)

Expand Down Expand Up @@ -77,7 +78,7 @@ def format(self):
class QueryExecuteErrorResponseEvent(object):
def __init__(self, parameters):
inner_params = parameters[u'error']
self.error_message = inner_params[u'message'] if u'message' in inner_params else ''
self.exception_message = inner_params[u'message'] if u'message' in inner_params else ''
self.error_code = inner_params[u'code'] if u'code' in inner_params else ''


Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Content-Length: 40

{"jsonrpc":"2.0","id":"1","result":true}Content-Length: 645

{"jsonrpc":"2.0","method":"connection/complete","params":{"ownerUri":"connectionservicetest","connectionId":"6fd410b5-cd48-487a-bc9c-90569e749772","messages":null,"errorMessage":null,"errorNumber":0,"serverInfo":{"serverMajorVersion":12,"serverMinorVersion":0,"serverReleaseVersion":2269,"engineEditionId":3,"serverVersion":"12.0.2269.0","serverLevel":"RTM","serverEdition":"Enterprise Edition (64-bit)","isCloud":false,"azureVersion":0,"osVersion":"Windows NT 6.3 <X64> (Build 9600: ) (Hypervisor)\n","machineName":"BRO-HB"},"connectionSummary":{"serverName":"bro-hb","databaseName":"AdventureWorks2014","userName":"cloudsa"},"type":"Default"}}Content-Length: 205

{"jsonrpc":"2.0","id":"2","error":{"code":0,"message":"SQL Execution error: A fatal error occurred.\r\n\tIncorrect syntax was encountered while select * from [HumanResources.Department was being parsed."}}
2 changes: 2 additions & 0 deletions mssqlcli/jsonrpc/contracts/tests/generatequerybaseline.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ def generate_query_baseline(file_name):
writer = json_rpc_client.JsonRpcWriter(tools_service_process.stdin)
writer.send_request(u'connection/connect', parameters, id=1)

time.sleep(2)

parameters = {u'OwnerUri': u'connectionservicetest',
u'Query': "select * from HumanResources.Department"}
writer.send_request(u'query/executeString', parameters, id=2)
Expand Down
62 changes: 50 additions & 12 deletions mssqlcli/jsonrpc/contracts/tests/test_executestring.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,50 @@ def test_query_execute_response_AdventureWorks2014(self):

request = queryservice.QueryExecuteStringRequest(
2, rpc_client, parameters)
self.verify_query_response(request=request)
self.verify_query_response(request=request,
expected_message_event=1,
expected_complete_event=1,
expected_batch_summaries=1,
expected_result_set_summaries=1,
expected_row_count=16)
rpc_client.shutdown()

def verify_query_response(self, request):
def test_malformed_query_AdventureWorks2014(self):
"""
Verify a failed query execute response for "select * from [HumanResources.Department"
"""

with open(self.get_test_baseline(u'malformed_query_adventureworks2014.txt'), u'r+b', buffering=0) as response_file:
request_stream = io.BytesIO()
rpc_client = json_rpc_client.JsonRpcClient(
request_stream, response_file)
rpc_client.start()
# Submit a dummy request.
parameters = {u'OwnerUri': u'connectionservicetest',
u'Query': "select * from [HumanResources.Department"}

request = queryservice.QueryExecuteStringRequest(
2, rpc_client, parameters)
self.verify_query_response(request=request,
expected_complete_event=1,
expected_error_count=1)
rpc_client.shutdown()

def verify_query_response(self,
request,
expected_message_event=0,
expected_complete_event=0,
expected_batch_summaries=0,
expected_result_set_summaries=0,
expected_row_count=0,
expected_error_count=0):
message_event = 0
complete_event = 0
batch_summaries = 0
result_set_summaries = 0
# For the executed query expected row count is 16
row_count = 0
error_count = 0

request.execute()
time.sleep(1)

Expand All @@ -50,18 +84,22 @@ def verify_query_response(self, request):
if response:
if isinstance(response, queryservice.QueryMessageEvent):
message_event += 1
elif isinstance(response, queryservice.QueryExecuteErrorResponseEvent):
error_count += 1
elif isinstance(response, queryservice.QueryCompleteEvent):
complete_event += 1
batch_summaries = len(response.batch_summaries)
result_set_summaries = len(
response.batch_summaries[0].result_set_summaries)
row_count = response.batch_summaries[0].result_set_summaries[0].row_count
if hasattr(response, 'batch_summaries'):
batch_summaries = len(response.batch_summaries)
result_set_summaries = len(
response.batch_summaries[0].result_set_summaries)
row_count = response.batch_summaries[0].result_set_summaries[0].row_count

self.assertEqual(message_event, 1)
self.assertEqual(complete_event, 1)
self.assertEqual(batch_summaries, 1)
self.assertEqual(result_set_summaries, 1)
self.assertEqual(row_count, 16)
self.assertEqual(message_event, expected_message_event)
self.assertEqual(complete_event, expected_complete_event)
self.assertEqual(batch_summaries, expected_batch_summaries)
self.assertEqual(result_set_summaries, expected_result_set_summaries)
self.assertEqual(row_count, expected_row_count)
self.assertEqual(error_count, expected_error_count)

def get_test_baseline(self, file_name):
"""
Expand Down
8 changes: 4 additions & 4 deletions mssqlcli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ def connect(self, database='', server='', user='', port='', passwd='',
connection_timeout=connection_timeout,
application_intent=application_intent,
multi_subnet_failover=multi_subnet_failover,
packet_size=packet_size,**kwargs)
packet_size=packet_size, **kwargs)

if not self.mssqlcliclient_query_execution.connect():
click.secho(
Expand Down Expand Up @@ -521,8 +521,8 @@ def _evaluate_command(self, text):
click.secho(u'No connection to server. Exiting.')
exit(1)

for rows, columns, status, sql, is_error in self.mssqlcliclient_query_execution.execute_multi_statement_single_batch(
text):
for rows, columns, status, sql, is_error in \
self.mssqlcliclient_query_execution.execute_multi_statement_single_batch(text):
total = time() - start
if self._should_show_limit_prompt(status, rows):
click.secho('The result set has more than %s rows.'
Expand Down Expand Up @@ -566,7 +566,7 @@ def _evaluate_command(self, text):
meta_changed = meta_changed or has_meta_cmd(text)

return output, MetaQuery(
sql, all_success, total, meta_changed, db_changed, path_changed, mutated)
text, all_success, total, meta_changed, db_changed, path_changed, mutated)

def _handle_server_closed_connection(self):
"""Used during CLI execution"""
Expand Down
12 changes: 4 additions & 8 deletions mssqlcli/mssqlcliclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ def execute_multi_statement_single_batch(self, query):
if not sql:
yield None, None, None, sql, False
continue
for rows, columns, status, sql, is_error in self.execute_single_batch_query(sql):
yield rows, columns, status, sql, is_error
for rows, columns, status, statement, is_error in self.execute_single_batch_query(sql):
yield rows, columns, status, statement, is_error

def execute_single_batch_query(self, query):
if not self.is_connected:
Expand All @@ -139,18 +139,14 @@ def execute_single_batch_query(self, query):
while not query_request.completed():
query_response = query_request.get_response()
if query_response:
if isinstance(query_response, queryservice.QueryExecuteErrorResponseEvent):
yield self.tabular_results_generator(column_info=None, result_rows=None,
query=query, message=query_response.error_message,
is_error=True)
elif isinstance(query_response, queryservice.QueryMessageEvent):
Copy link
Contributor

@abhisheksinha89 abhisheksinha89 Jan 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are we deleting this? We need it to hamdle Error response events #Closed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh ok I see now what happened. Thanks


In reply to: 161077826 [](ancestors = 161077826)

if isinstance(query_response, queryservice.QueryMessageEvent):
query_messages.append(query_response)
else:
sleep(time_wait_if_no_response)

if query_response.exception_message:
logger.error(u'Query response had an exception')
yield self.tabular_results_generator(
return self.tabular_results_generator(
column_info=None,
result_rows=None,
query=query,
Expand Down