implement show hosts #80

ayyt · 2019-01-11T07:34:38Z

implement show hosts syntax

ShowAllHostExecutor::execute interface not implement,
when StorageClient fininshed, then implement this interface

nebula-community-bot · 2019-01-11T07:35:03Z

Can one of the admins verify this patch?

sherman-the-tank · 2019-01-13T01:01:54Z

jenkins go

nebula-community-bot · 2019-01-13T01:07:33Z

Build succeeded.

dutor · 2019-01-14T05:09:04Z

src/parser/parser.yy

@@ -663,6 +664,13 @@ update_edge_sentence
    }
    ;

+show_sentence


We would support to SHOW many kinds of informations in future, so what's your plan on this, one sentence/executor for each kind of info?

I think one kind show command should correspond to one sentence/executor.

That would be tooo many.

Yes, this makes sense. I use one sentence/executor to all kinds of show command。

dutor · 2019-01-14T05:10:04Z

src/parser/parser.yy

@@ -685,6 +693,7 @@ sentence
    | piped_sentence {}
    | assignment_sentence {}
    | mutate_sentence {}
+    | show_sentence {}


IMO. this belongs to maintainance_sentence.

At first, I also think so. But maintainance_sentence include define_tag_sentence, define_edge_sentence, alter_tag_sentence ， alter_edge_sentence， describe_tag_sentence， describe_edge_sentence。

These sentences should be different from show sentence.

here I move show_sentence to belongs to maintainance_sentence.

dutor · 2019-01-14T05:11:41Z

src/parser/scanner.lex

@@ -19,95 +19,101 @@ static constexpr size_t MAX_STRING = 4096;
 %}

 %x STR
-
-GO                          ([Gg][Oo])
+/* These tokens are kept sorted for human lookup */


I dont think this rationale is strong enough to make these order changes.

when I add a token, firstly I have to make sure the token doesn't exist before.
when these tokens are kept sorted, It's easy to find whether a token exists.

Yeah, but it's still not a strong reason, since every editor has searching in support. Besides, if you define a duplicated token, the generator would tell you.

Make sense.
Keep the tokens sorted to easy search and maintain. I also referred to other database.

I will rollback these code.

dutor · 2019-01-14T05:12:30Z

src/parser/scanner.lex

+FALSE                       ([Ff][Aa][Ll][Ss][Ee])
+FROM                        ([Ff][Rr][Oo][Mm])
+GO                          ([Gg][Oo])
+HOST                        ([Hh][Oo][Ss][Tt])


Should it be plural?

Good catch 👍

I think The SQL “show hosts” should be better。

dutor · 2019-01-14T05:26:50Z

src/parser/test/ScannerTest.cpp

        CHECK_SEMANTIC_VALUE("\"\\110ello\"", TokenType::STRING, "Hello"),
+        CHECK_SEMANTIC_VALUE("\"\110ello\"", TokenType::STRING, "Hello"),
+
+        CHECK_SEMANTIC_VALUE("\"\110 \"", TokenType::STRING, "H "),


I have no idea what the following lines are testing for. They seems covered by above tests already.

here test the string with multiple escape characters.

ayyt · 2019-01-14T06:30:15Z

Thanks for your review @sherman-the-tank @dutor

dutor · 2019-01-14T06:42:53Z

src/parser/parser.yy

@@ -663,6 +664,13 @@ update_edge_sentence
    }
    ;

+show_sentence
+    : KW_SHOW KW_ALL KW_HOST {


What about just SHOW HOSTS?

Yeah， I couldn't agree more。 mysql has similar sql ， for example show databases, show tables, show variables.👍

ayyt · 2019-01-14T10:37:15Z

update the code.

dutor · 2019-01-15T03:23:42Z

src/parser/ShowSentences.h

+
+class ShowSentence final : public Sentence {
+public:
+    explicit ShowSentence(std::string cmdstr) {


Why bother to use a string instead of the enum directly?

Yeah, I use enum directly.

dutor · 2019-01-15T03:25:22Z

src/parser/ShowSentences.h

+public:
+    explicit ShowSentence(std::string cmdstr) {
+        kind_ = Kind::kShow;
+        if (cmdstr.compare("hosts") == 0)


FYI. cmdstr == "hosts" works.

styling: always put the if/else block into braces.

make sense. I will update

dutor · 2019-01-15T03:27:58Z

src/parser/scanner.lex

@@ -108,6 +111,8 @@ OCT                         ([0-7])
 {OVERWRITE}                 { return TokenType::KW_OVERWRITE; }
 {TRUE}                      { yylval->boolval = true; return TokenType::BOOL; }
 {FALSE}                     { yylval->boolval = false; return TokenType::BOOL; }
+{SHOW}                      { return TokenType::KW_SHOW; }
+{HOSTS}                     { return TokenType::KW_HOSTS; }


Maybe you need add tests on these two keywords in ScannerTest?

Good catch 👍

dutor · 2019-01-16T05:34:39Z

@steppenwolfyuetong Could you please perform a rebase from upon upstream/master?

ayyt · 2019-01-16T05:54:27Z

@steppenwolfyuetong Could you please perform a rebase from upon upstream/master?

Rebased. Please review.

dutor · 2019-01-16T05:57:29Z

Jenkins go

nebula-community-bot · 2019-01-16T06:05:20Z

Build succeeded.

dutor

Thanks for your contribution!

Co-authored-by: Aiee <[email protected]>

dutor reviewed Jan 14, 2019

View reviewed changes

dutor reviewed Jan 15, 2019

View reviewed changes

ayyt changed the title ~~implement show all host~~ implement show hosts Jan 15, 2019

yuetong added 5 commits January 15, 2019 23:49

implement show all host

f4b83b8

endImprove the code of show hosts

185d733

endimprove show hosts

bf1d053

endimprove show hosts

fb2a621

use std::move

904b7ad

dutor approved these changes Jan 16, 2019

View reviewed changes

dutor merged commit 03d5be3 into vesoft-inc:master Jan 16, 2019

dutor deleted the show_all_host branch January 16, 2019 06:09

yixinglu pushed a commit to yixinglu/nebula that referenced this pull request Feb 16, 2020

Implement show hosts (vesoft-inc#80)

ee91935

tong-hao pushed a commit to tong-hao/nebula that referenced this pull request Jun 1, 2021

Implement show hosts (vesoft-inc#80)

93e041c

yixinglu pushed a commit to yixinglu/nebula that referenced this pull request Mar 21, 2022

Fix version display in docker (vesoft-inc#80)

788caae

Co-authored-by: Aiee <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement show hosts #80

implement show hosts #80

ayyt commented Jan 11, 2019 •

edited

Loading

nebula-community-bot commented Jan 11, 2019

sherman-the-tank commented Jan 13, 2019

nebula-community-bot commented Jan 13, 2019

dutor Jan 14, 2019

ayyt Jan 14, 2019

dutor Jan 14, 2019

ayyt Jan 14, 2019

dutor Jan 14, 2019

ayyt Jan 14, 2019

dutor Jan 14, 2019

ayyt Jan 14, 2019

dutor Jan 14, 2019

ayyt Jan 14, 2019

dutor Jan 14, 2019

ayyt Jan 14, 2019

ayyt Jan 14, 2019

dutor Jan 14, 2019 •

edited

Loading

ayyt Jan 14, 2019

ayyt commented Jan 14, 2019

dutor Jan 14, 2019

ayyt Jan 14, 2019

ayyt commented Jan 14, 2019

dutor Jan 15, 2019

ayyt Jan 15, 2019

dutor Jan 15, 2019

ayyt Jan 15, 2019

dutor Jan 15, 2019 •

edited

Loading

ayyt Jan 15, 2019

dutor commented Jan 16, 2019

ayyt commented Jan 16, 2019

dutor commented Jan 16, 2019

nebula-community-bot commented Jan 16, 2019

dutor left a comment

implement show hosts #80

implement show hosts #80

Conversation

ayyt commented Jan 11, 2019 • edited Loading

nebula-community-bot commented Jan 11, 2019

sherman-the-tank commented Jan 13, 2019

nebula-community-bot commented Jan 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dutor Jan 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayyt commented Jan 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayyt commented Jan 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dutor Jan 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dutor commented Jan 16, 2019

ayyt commented Jan 16, 2019

dutor commented Jan 16, 2019

nebula-community-bot commented Jan 16, 2019

dutor left a comment

Choose a reason for hiding this comment

ayyt commented Jan 11, 2019 •

edited

Loading

dutor Jan 14, 2019 •

edited

Loading

dutor Jan 15, 2019 •

edited

Loading